facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.08k stars 1.86k forks source link

Flowtype not maintaining object type consistency #3708

Open vicapow opened 7 years ago

vicapow commented 7 years ago

Apologies if I'm not using the correct terminology. As best I can tell, this seems to be an issue with Flow not maintaining that the type of key is the same type in both cases of the read and write of the property.

/* @flow */
type TypeA = {foo: number, bar: string};
const a = {foo: 0, bar: '1'};
Object.keys(a).forEach((key: $Keys<TypeA>)=> {
  a[key] = a[key]; // unexpected error
  if (key === 'foo' || key === 'bar') {
    a[key] = a[key]; // also unexpected error
  }
  if (key === 'foo') {
    a[key] = a[key]; // works
  }
});
nmote commented 7 years ago

It will be easier to help you if you include the error text.

vicapow commented 7 years ago

oh, sure

7:   a[key] = a[key];
              ^ number. This type is incompatible with
3: type TypeA = {foo: number, bar: string};
                                   ^ string
7:   a[key] = a[key];
              ^ string. This type is incompatible with
3: type TypeA = {foo: number, bar: string};
                      ^ number

or: https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVAXAngBwKZgAqueAgmALxgDeUccAXGAHYCuAtgEZ4BOANGE4BDHkwDOGHgEtmAcwC+AblQBjOMwlghlGnUZgADAOGiwAcgCMZpeiitmKjFPVg7DgDykmAEgDSeLDF3YnxSAD4wgAohJhCyAQBrAKZSAEoaVDAtAG0krABdHSFcgPzleXQAeU4AKzxHADo8sWjUhroeAFEhFQALSMi8n39A4JJw1IowjKy3FWjEgNTy5dQgA

nmote commented 7 years ago

I see. I think you understand the issue. Flow doesn't keep track of the fact that key is the same in both places. That's a more complex analysis and I don't know what it would take to implement it. I think there are lots of opportunities for Flow to do a more precise analysis so I'm not sure when or if we will address this case.

vicapow commented 7 years ago

In cases like this, is the ideal workaround to use something like // $FlowFixMe or maybe an additional type annotation we can add?

vkurchatkin commented 7 years ago

@vicapow ideally, you shouldn't write a[key] = a[key] 😄

vicapow commented 7 years ago

@vkurchatkin Sorry! I realize it's a silly example but from looking at the other issues, it seemed like folks were looking for simplified reproduction steps. I may have taken that too far :P For more context and to help motivate the issue, I was trying to implement a way to "reset" fields in a form without mutating the original form.

/* @flow */

type Input<T> = {input: T, error: string};
type Form = {foo: Input<number>, bar: Input<string>};
let form = {foo: {input: 0, error: 'a'}, bar: {input: '1', error: 'b'}};

function resetInfo(a: Form): Form {
    Object.keys(a).forEach((key: $Keys<Form>)=> {
        a[key] = {...a[key], info: ''};
    });
    return {...a};
}
form = resetInfo(form);

Should see a similar error:

9:      a[key] = {...a[key], info: ''};
              ^ object literal. This type is incompatible with
4: type Form = {foo: Input<number>, bar: Input<string>};
                     ^ object type
9:      a[key] = {...a[key], info: ''};
              ^ object literal. This type is incompatible with
4: type Form = {foo: Input<number>, bar: Input<string>};
                                         ^ object type

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVAXAngBwKZgCSAdjgK4YA8AKgHxgC8YA3gJakUBcY1ANGHgBOguIO4BnDIPYBzAL4BuTLgIAxUQFtGLKHDjcS5KsTIaARkNr8zAQzFEOVSdOIzai1DDwYwuwVqZmXX0WdiNuAAZ+IRF7AHIbOLlrO242R244gEY46OFRTLMkj1QoMmIAYwxWOGIwQTxxbxJdAAobbnV-AEpOzRZUAEgAeTMAKzwqgDoAazwscXbuqb8AURsKgAtW1rmsbgASAGl58UoujVpuhnpmIcGbAG09gF1tZinPp9f+dl1MpJKQZybpAhoYMiCOofL4eOSlfpMBpNDAtOCtPwaUFAA

jdeal commented 6 years ago

Here's a similar example:

const obj = {
  foo() {},
  bar: true
}

Object.keys(obj).forEach(key => {
  if (typeof obj[key] === 'function') {
    obj[key] = obj[key].bind(obj);
  }
});
8:     obj[key] = obj[key].bind(obj);
                  ^ Cannot assign `obj[key].bind(...)` to `obj[key]` because function [1] is incompatible with boolean [2].
References:
2:   foo() {},
        ^ [1]
3:   bar: true          ^ [2]

https://flow.org/try/#0MYewdgzgLgBCBGArGBeGBvAUDGAzEIAFAJQYC+ANNjPAIYBOAXDFPQK4CmmZmmA8kg7AoAOgDWHAJ4RCCRMRH56AUVrAAFoQmTUAPgzUAlrhiEokgA4cQJuQG1tAXVQo0Aclxswww+DeksHBx7J1Q4JAcpRxF4QzAAE1kkYgBuah4yVKA

Of course, I can bind obj.foo directly, but that defeats the purpose here: to loop over arbitrary/unknown properties and only bind the functions.