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

Incorrect mixed type from value of Object.entries #5838

Closed
patrickpei opened this issue Feb 19, 2018 · 8 comments
Closed

Incorrect mixed type from value of Object.entries #5838

patrickpei opened this issue Feb 19, 2018 · 8 comments

Comments

@patrickpei
Copy link

patrickpei commented Feb 19, 2018

Flow assumes that value as returned by [key, value] for Object.entries is mixed. Not really sure where to start looking but willing to check it out / try to help!

const obj: {
  	[string]: number
} = {
	a: 1,
  	b: 2,
  	c: 3
};
for (const [char, num] of Object.entries(obj)) {
	const copy: number = num;
}

Error:

11: 	const copy: number = num;
                          ^ Cannot assign `num` to `copy` because mixed [1] is incompatible with number [2].
References:
[LIB] static/v0.66.0/flowlib/core.js:35:     static entries(object: any): Array<[string, mixed]>;
                                                                                         ^ [1]
11: 	const copy: number = num;
                 ^ [2]

Try Flow

@spudly
Copy link

spudly commented May 10, 2018

Problem is here: https://github.com/facebook/flow/blob/master/lib/core.js#L48

@houshuang
Copy link

See #2221. I'm currently using the values/entries code in the bottom of this file to great effect. https://github.com/chili-epfl/FROG/blob/develop/frog-utils/src/index.js#L285-L293

@mrkev
Copy link
Contributor

mrkev commented May 14, 2018

Would replacing the current declaration linked to by @spudly with this work? Anyone care to test it out and submit a PR?

@jcready
Copy link
Contributor

jcready commented Jun 4, 2018

No @mrkev, the definitions for values and entries that @houshuang linked to are unsound. Here is a simple example:

const entries = <T>(obj: { [string]: T }): Array<[string, T]> => {
  const keys: string[] = Object.keys(obj);
  return keys.map(key => [key, obj[key]]);
};

const values = <T>(obj: { [string]: T }): Array<T> => {
  const keys: string[] = Object.keys(obj);
  return keys.map(key => obj[key]);
};

function test(val: { foo: string }) {
  values(val).forEach(v => {
    v.toLowerCase();
  });
  entries(val).forEach(([ k, v ]) => {
    v.toLowerCase();
  });
}

test({ foo: 'x', bar: 123 });

This going to pass typechecking, but fail at runtime.

@houshuang
Copy link

houshuang commented Jun 4, 2018 via email

@mrkev
Copy link
Contributor

mrkev commented Jun 5, 2018

@jcready ah true, inexact objects back at it again

facebook-github-bot pushed a commit that referenced this issue Dec 5, 2022
…ictionary (mixed otherwise)

Summary:
User request from Dec 2021, May 2021, Dec 2020, Apr 2020

For `Object.entries`, keys is typed exactly like it is for `Object.keys`

For both, values is typed as such:
- Dictionaries: the value of the dictionary
- Objects & Instances: `mixed` - this avoids created non-performant union types

Error diff analysis:
- Previously, `Object.entries` returned `string` for the key type, rather than having the same behavior as `Object.keys`. We unsafely allow property read/writes with `string`.
- Error code for suppressions has changed in some places from `incompatible-call` to `incompatible-use`
- Many errors have simply moved (previously error on usage of `mixed`, now erroring on still invalid but more accurate type)

After WWW diff D41527152, errors are only around ~500.

Alternative 1 was to create a new utility type `$DictValues` and type the `Object.values` and `Object.entries` lib defs with this. Cons of this approach are introducing a new lib def, and the fact that `Object.entries` keys definition would differ from `Object.keys`, since `$Keys` and `Object.keys` are implemented differently.

Changelog: [errors] Instead of `mixed`, type the result of `Object.values` and `Object.entries` on a dictionary to be the dictionary values, and `Object.entries` keys to behave like `Object.keys`

Closes #2174, #2221, #4771, #4997, #5838

Reviewed By: jbrown215

Differential Revision: D35710131

fbshipit-source-id: 3163bc02dfc7cb70a5d6c0ba7da574a793672421
@gkz
Copy link
Member

gkz commented Dec 5, 2022

With above commit, 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 as completed Dec 5, 2022
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

7 participants