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

Add generic to Object.entries #7717

Closed

Conversation

Dimitreee
Copy link

@Dimitreee Dimitreee commented May 12, 2019

In my humble opinion there is one strong problem, in Object.entries() typings .
Me and many other developers cannot use this perfect API because we should "fight" with this mixed Tuple second value type, and as the result, flow wins, this method keeps being unused.

Here is simple implementation of the problem

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@lyleunderwood
Copy link
Contributor

This is unsound given inexact objects.

@goodmind
Copy link
Contributor

How many PRs adding this were made? Same for Object.keys, maybe docs should explain soundness better?

@lyleunderwood
Copy link
Contributor

It would be nice if Object.values and Object.entries could be re-addressed when the switch to exact-by-default happens.

@Dimitreee
Copy link
Author

Dimitreee commented May 13, 2019

@lyleunderwood @goodmind
I prefer to manually control the "Types" with my hands and work with the "Type" I needs, and not put up with those "mixed". It sounds horrible that i should reassign "Types" of default API to "any".

By the way, @goodmind, why dont you use "Exact" ?

@goodmind goodmind added the Library definitions Issues or pull requests about core library definitions label May 17, 2019
@nnmrts
Copy link
Contributor

nnmrts commented Aug 29, 2019

@goodmind Just be honest and say you never use Object.values/.entries/.keys together with flow and that's why you just don't see a problem with that.

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

Same thing with Object.values btw...

Again:
This. Is. A. Bug.

I don't know why you refuse to understand that. You always bring up the same argument, "this breaks with inexact objects", well, yeah, thank you very much, we know that too.

There are a lot of good suggestions by a lot of people, a lot of pull requests and issues were opened, but they all get closed for no reason, one after another. For more than 3 years now. And now the most popular typechecking library isn't able to properly typecheck objects. Congrats.

And we can't even override flowlib definitions? What the hell?

TIHI.

@goodmind
Copy link
Contributor

goodmind commented Aug 29, 2019

@nnmrts you don't need to be so negative here, I understand your frustration, I'm just a user too. This just goes against Flow goal to be as sound as possible, Flow never had completeness in mind.
It's totally possible to make sound Object.keys/values/entries but my PR wasn't accepted too because "exact constraint is flawed idea"

EDIT: Collaborator badge just means I have rights to label issues, I don't work for Facebook or Flow team related

@nnmrts
Copy link
Contributor

nnmrts commented Aug 29, 2019

Ye, sorry. I don't want to attack anyone personally here, but the fact that flow is missing this feature is just mind-boggling to me.
So, "exact constraint is flawed idea"? Why? Did anyone actually explained why? And/Or provided another solution?

Because just not fixing it and not letting others try to fix it, is not a solution, Facebook.

Every other library I know would give you an option to opt-in for a feature, when the developers think the idea of it is "flawed" but the feature still "just works" for most of the users.

Flow never had completeness in mind

That's not a completeness issue tho, sorry. Flow is described as "a static typechecker for JavaScript." and this is a direct quote from the website:

Flow is designed to understand idiomatic JavaScript. It understands common JavaScript patterns and many of the weird things we JavaScript developers love to do.

Turns out it doesn't understand common JavaScript patterns.

I would understand the completeness argument if we are talking about async/await stuff, experimental es7 stuff, generator stuff, even class stuff. No sane person would complain about that, because it's acceptable that a typechecker can't understand language syntax that was basically released yesterday. But:

  • The Object.values and Object.entries definitions were finalized in the specification for ECMAScript2017 (!).
  • The Object.keys and Array.prototype.map definitions were finalized in the specification for ECMAScript 5.1 (June 2011!!!)
  • Object.values(obj) is just a shorthand for Object.keys(obj).map((key) => obj[key])
  • Object.entries(obj) is just a shorthand for Object.keys(obj).map((key) => [key, obj[key]])

A typechecker that doesn't support basic, 8 years old features of a language isn't "incomplete", it's useless.

@goodmind
Copy link
Contributor

goodmind commented Aug 29, 2019

What I'm referring to
#6927 (comment)
#2221 (comment)

@nnmrts
Copy link
Contributor

nnmrts commented Aug 29, 2019

@goodmind Thanks, I've already read those comments, however we got no reaction to them.

This whole problem is ignored since 2016, at least here on GitHub.

@gkz
Copy link
Member

gkz commented Dec 5, 2022

With b9aae51, object types that have an indexer and no explicit properties (e.g. {[string]: number}) have the type of their dictionary values returned by Object.values (e.g. Array<number>), and similarly with Object.entries, except the first element of the tuple works exactly like Object.keys (in this case, Array<[string, number]>)

@gkz gkz closed this Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Library definitions Issues or pull requests about core library definitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants