facebook / flow

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

Regression in invalid-computed-prop detection #9141

Closed Robloche closed 2 months ago

Robloche commented 2 months ago

Flow version: 0.235.1

Expected behavior

My code looks like this:

type Dict<T> = {| [string]: T |};
export type BoolDict = Dict<bool>;

Then, in another file, I have:

const bd: BoolDict = { foo: true };
const k = 'foo';
const {[k]: v} = bd;

Until flow-bin v0.234.0, no error was raised.

Actual behavior

Since I upgraded to flow-bin v0.235.1, the line const {[k]: v} = bd; raises the following error:

Cannot access object with computed property using string [1]. [invalid-computed-prop]
[1] k: string,

I tried to reproduce on flow.org/try but to no avail.

But I tried the following code, which does not raise any error:

export type BoolDict = {| [string]: bool |};

The other part remains unchanged:

const bd: BoolDict = { foo: true };
const k = 'foo';
const {[k]: v} = bd;

So my guess is it has something to do with the generic type, or the export, or a combination of both.

Thanks for your help.

SamChou19815 commented 2 months ago

This is due to the fact that things like

declare const obj: {foo: string};
declare const key: string;
obj[key]

are never safe, and just returns any before. In the latest Flow version, we choose to start loudly error.

gkz commented 2 months ago

I tried your initial example, locally over two files, and was not able to reproduce the error.

Robloche commented 2 months ago

I struggled a bit but I eventually understood what I did wrong. Thanks for your answer, @SamChou19815!

Robloche commented 2 months ago

Well, there's still something bothering me. What about this example:

enum MyEnum {
    ValFoo = 'foo',
    ValBar = 'bar'
}

declare const obj: {foo: string, bar: string};
declare const key: string;
obj[MyEnum.ValFoo as string]

Why doesn't it work, since MyEnum.ValFoo as string is indeed the string "foo"?

Robloche commented 2 months ago

In fact, I still don't understand the difference between (1):

type Dict<T> = {| [string]: T |};
export type BoolDict = Dict<bool>;

and (2):

export type BoolDict = {| [string]: bool |};

The following code does not work with (1) but is OK with (2):

const bd: BoolDict = { foo: true };
const k = 'foo';
const {[k]: v} = bd;
SamChou19815 commented 1 month ago

The enum example is an issue with enum feature. The right solution is to support enum key rather than continue to allow this unsafe access.

In fact, I still don't understand the difference between (1):

type Dict<T> = {| [string]: T |};
export type BoolDict = Dict<bool>;

and (2):

export type BoolDict = {| [string]: bool |};

The following code does not work with (1) but is OK with (2):

const bd: BoolDict = { foo: true };
const k = 'foo';
const {[k]: v} = bd;

I am still unable to reproduce this? Can you provide a try-flow link?

Robloche commented 1 month ago

Unfortunately, I can't. I think it has something to do with export & import and only fails when the code is spread across multiple files, because it works correctly in try-flow.