flow
flow copied to clipboard
Use SingletonStrT, SingletonNumT, SingletonBoolT in case of const literal declarations
Revived https://github.com/facebook/flow/pull/4175 Fixes #2639
@mrkev probably
Let's check this out
I'm seeing issues similar to this:
const foo = {
bar: 10,
baz(x: number) {
foo.bar = x; // Error, number incompatible with number
},
}
Also tagging @nmote so this PR get some visibility from someone in the team.
@mrkev I'm not sure how this is related, because it only uses affects const primitive literals, not object literals properties, I'll check
Maybe you mean something like this
const bar = 10;
const foo = {
bar,
baz(x: number) {
foo.bar = x; // Cannot assign `x` to `foo.bar` because number [1] is incompatible with number
},
}
Yeah, totally bug, I should replace reason of DefT
Now this errors correctly
Cannot assign `x` to `foo.bar` because number [1] is incompatible with number literal `10` [2]
@mrkev do you think this shouldn't error at all and property type should be widened to number?
It's interesting that in TypeScript if type is inferred then it's widened, but if it's specified by user it isn't
const str = 'string';
const num = 200;
const bool = true;
const str1: 'string' = 'string';
const num1: 200 = 200;
const bool1: true = true;
const foo = {
str,
num,
bool,
str1,
num1,
bool1,
method() {
foo.str = 'any string' // no error
foo.num = 300 // no error
foo.bool = false // no error
foo.str1 = 'any string' // error
foo.num1 = 300 // error
foo.bool1 = false // error
}
}
I think it depends if this code is bug or not https://flow.org/try/#0MYewdgzgLgBAYgeQQLhgIkQtMC87NoDcAUMADYCGEEMAQhQE4wDexMMUAngA4Cmu8JCXYALXmTIgAFAEoWbdhxEBLCADoufAWjDLgvIgoC+xE6EiwARowGt2m3qkwAaBWInS5dxdYYae-Hg6egbCMCZGhEA
I'm now not sure what the cases for this are, refinements and switch seems to work fine with just regular const
I think it depends if this code is bug or not https://flow.org/try/#0MYewdgzgLgBAYgeQQLhgIkQtMC87NoDcAUMADYCGEEMAQhQE4wDexMMUAngA4Cmu8JCXYALXmTIgAFAEoWbdhxEBLCADoufAWjDLgvIgoC+xE6EiwARowGt2m3qkwAaBWInS5dxdYYae-Hg6egbCMCZGhEA
I think what typescript does falls in the right direction, and would personally err on the side of widening, though this is a good question to bring up on the Discord channel for discussion IMO.
So this is stalled because we didn't found out how to widen literals to string in mutable locations. This would definitely need another kind of SingletonStrT, and more handling related to variance?
@mrkev what you think about implementing const assertions instead? https://devblogs.microsoft.com/typescript/announcing-typescript-3-4/#const-assertions
EDIT: nvm, same problems with inference https://flow.org/try/#0DYUwLgBAthC8EAoDaBGANBATBgzAXQC4JUNsJ8BKAKCpnhSqA
The only working thing is fixing typeof directly, since this is most used case of getting literal type from string
Related issue https://github.com/facebook/flow/issues/7961
@mrkev
declare var foo: {+a: 'str'}
declare var foo_a: 'str';
const bar = {
a: foo.a,
b: foo_a,
method() {
bar.b = 'bar' // error
bar.a = 'bar' // no error
}
}
https://flow.org/try/#0CYUwxgNghgTiAEA3W8BmB7dAueBvA1FDgOQDOALjMQL4BQoksCyMamA+kfGZcQNy1aYdADsK8AEYoAvHlrx4XDOgB0UADTzJOZZ00KAtiHIALdMAAUASjkKFUmConxZxB8S33Yal93da6aiA
Kinda strange, how can we get bar.b work the same as bar.a?
@mrkev I solved this problem, should I add more tests?
This actually makes it inconsistent with let, hmm
https://flow.org/try/#0PTAEAEDMBsHsHcBQjoFMAupKgLygOST6LZ4CMA3CbqOgE4CuqViAxrAHYDOmsNA3lgBcBIqAC+ydt0wAjAIZ0BiUKAAeIwvgA0K0LBGwAdJESSSDDq3QBLTuoAUASlD89CukbU1KoEKBsrTi4bHlQOTHgbdAALUBiEUAADNHQk0HhYOgBrLndFLxp6Jgo-MEDpELCIjOi4hPhk1PTMnLzVfM8+clL-CuDQ9HDIutB5WVgAN1ROo27aRmYygKDuQeHa2LGJ6bNkDznEA7UgA
but it is separate issue