Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Another Object.{entries,values} attempt #6385

Closed
wants to merge 2 commits into from

Conversation

deecewan
Copy link
Contributor

@deecewan deecewan commented May 30, 2018

I've read through a whole load of posts surrounding this topic, but can't quite get my head around exactly why Object.keys works in every case, but Object.entries and Object.values are unsafe.

Most importantly, why Object.keys(object).map(k => object[k]) can correctly (and safely, apparently) pull out the values from a { [string]: T }, but we can't have the same happen from Object.values.

So I've opened a PR, because then Cunningham's Law says I'll get an answer (or maybe this is correct).

I've added tests, but I'm almost certain that these don't cover every case.

I've split this into two commits so that the updates to the test files don't get in the way of the code I've added.

@mrkev
Copy link
Contributor

mrkev commented Jun 14, 2018

hahha I gochu-- gonna fufill Cunningham's law here (didn't know it was a thing, but I guess it kinda is true a lot of times?).

The issue is width subtyping, which is necessary and shouldn't be deemed incorrect, but makes things a little tricky.

Ergo, take this example:

// @flow

declare function values<T>(object: { [any]: T }): Array<T>;

type T = { a: string }
const obj = {a: "hello", b: 4}

function f (obj: T) {
 return values(obj)
}

const foo = f(obj)

This typechecks, and with this definition of values it's assumed to return type Array<string>, but at runtime foo will include a number. There might be a way to make this work, but it'd probably require some deeper changes to flow itself and not just the type definitions.

Object.prototype.entries is similar in that it uses the type of the value. Object.prototype.keys works because keys are always strings. Thanks for giving this a shot though, appreciate the thought and effort!

@mrkev mrkev closed this Jun 14, 2018
@deecewan
Copy link
Contributor Author

Thanks for the explanation @mrkev. Width subtyping is definitely necessary, and I hadn't considered it here.

@deecewan
Copy link
Contributor Author

beating a dead horse here, but if there was a way to say 'if the type is an exact type, use this libdef', we could to this, right? the would prevent the width subtyping issue and let people who were using exact types use a stricter, more helpful definition of values.

pseudocode:

declare function values<T>(object: $IsExactType<{ [any]: T }>): Array<T>;
declare function values(object: Object): Array<mixed>;

type T = {a: string}
type C = {|a: string|}

function f (obj: T) {
  return values(obj)
}

function fc(obj: C) {
  return values(obj);
}

const foo: Array<T> = f({a: "hello", b: 4}); // fails, because T is not an exact type
const bar: Array<C> = fc({a: "hello"}); // passes because C is an exact type.

@goodmind
Copy link
Contributor

@deecewan there is a way, but it isn't correct, technically exact types doesn't have supertype

@d4kris
Copy link

d4kris commented Nov 20, 2020

Why does that example type check without errors though? I would have expected a type error on the call to f(obj) - since obj is not type T.
I think it should be up to me as a developer to assure that if I have an object declared as type T, I should make sure it actually adheres to that type definition. If flow could warn me about my indiscretions it would be great but the main takeaway is that values and entries could work again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants