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

Object.values() appears to be unusable with custom types. #2133

Closed
thisbejim opened this issue Jul 26, 2016 · 16 comments
Closed

Object.values() appears to be unusable with custom types. #2133

thisbejim opened this issue Jul 26, 2016 · 16 comments

Comments

@thisbejim
Copy link

In lib/core.js:

static values(object: any): Array<mixed>;

This method doesn't seem to be compatible with a custom type because it is defined as a mixed array.

Simple example:

types:

type Person = {
  name: string
}

export type People = Array<Person>;

function:

const getPeople = (): People => {
  // fetch data from an api for example
  const people = {
    john: {
      name: "John"
    },
    frank: {
      name: "Frank"
    }
  }
  return Object.values(people);
}

Which results in something like:

return Object.values(people); 
           ^^^^^^^^^^^ call of method `values`
export type People = Array<Person>;
                                ^^^^^ object type. This type is incompatible with. See: app/actions/types.js:75

Does this mean flow users need to rewrite utility functions like .values() to account for custom types?

@vkurchatkin
Copy link
Contributor

Object type implies that an instance has particular properties, but it doesn't mean that it doesn't have any other properties. So, mixed is the only safe option

@stryju
Copy link
Contributor

stryju commented Aug 10, 2016

@vkurchatkin well, it does if you annotate it:

type Person = {
  name: string,
};

type: People = {
  [key: string]: Person,
};

const people: People = {
  john: {
    name: "John"
  },
  frank: {
    name: "Frank"
  }
};

I'd follow this issue: #2174

@deecewan
Copy link
Contributor

deecewan commented Feb 1, 2018

@vkurchatkin I've seen that touted a couple of times, but the main problem is that the same functionality can be implemented in a 'type-safe' way using Object.keys.

type Person = { name: string };
type People = { [string]: Person };

const people = {
	john: { name: 'john'},
	frank: { name: 'frank' },
}

const fromValue: Array<Person> = Object.values(people); // fails
const fromKeys: Array<Person> = Object.keys(people).map(key => people[key]); // works

Here is the same code in the 'Try' page.

To my understanding, the fromKeys is essentially the same as the underlying implementation of fromValues (at least, that's how polyfill libs did it for a while). And I can't see how using keys can be any more sure of what 'values' will be present.

@ggregoire
Copy link

ggregoire commented Apr 17, 2018

Please reopen this issue.

The last example posted by @deecewan demonstrates how Object.values: mixed is wrong or not consistent.

@deecewan
Copy link
Contributor

deecewan commented May 29, 2018

given my understanding of how flow treats objects, I think this should be good as a libdef:

declare function values<T>(p: { [string]: T }): Array<T>;
declare function values(p: Object): Array<mixed>;

This way, if a known object is passed in (with only a single value type), it returns an array of the known values. If any other object type is passed in, it returns an array of mixed.

this repl shows it in action. Unless I'm missing something, this should work.

Edit: In fact, this same thing should work for Object.entries.

@mrkev
Copy link
Contributor

mrkev commented May 30, 2018

Hmm, this looks promising. I'd like to examine this more closely. PR @deecewan?

@deecewan
Copy link
Contributor

deecewan commented May 30, 2018

I started one earlier. There were a couple of issues.

I'll finish it up and put it up for comment in the morning.

Edit: #6385

@sibelius
Copy link

@deecewan this looks correct

@deecewan
Copy link
Contributor

It seems to work in most cases for values. For some reason, my implementation was no working correctly with entries. I'll look into it soon.

@nnmrts
Copy link
Contributor

nnmrts commented Aug 29, 2019

Why is this still closed?

Object.values resulting in Array<mixed> every time, no matter what, is a bug.
Object.entries resulting in Array<[string, mixed]> every time, no matter what, is a bug.

@nnmrts
Copy link
Contributor

nnmrts commented Aug 29, 2019

I just love it how there's now way to use Object.values with flow. It really just is unusable.

@deecewan
Copy link
Contributor

@nnmrts I just avoid values and entries and use Object.keys(...).map(...) instead

@nnmrts
Copy link
Contributor

nnmrts commented Aug 29, 2019

@deecewan Great! You could also just use instanceof every time!

Or even better, just use the // $FlowFixMe comment over every second line!

No, wait, even better, just typecheck by hovering over every variable in your IDE, that will do it!


Sorry but after over 3 years I'm expecting a little more than shitty workarounds for this, from a library developed by facebook. Whatever, sorry for ranting...

@goodmind
Copy link
Contributor

I think other issue already exists

@DnEgorWeb
Copy link

Really, why is this issue closed? "Avoid using Object.values(), use Object.keys() instead" is not a solution. Code should not depend on instruments like Flow, especially when we talk about native methods like Object.values().

@Robloche
Copy link

Why is this issue still closed?
Having Object.keys() not behaving like Object.values() and Object.entries() seems really awkward.

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

No branches or pull requests