Flow: Object access with arbitrary keys (object indexer) should return maybe type

[villesau]

try flow

type Obj = {[id: string]: {a: number}};

function fn(o: Obj, id: string): number {
    return o[id].a
}

As you see from the above example, Flow does not complain when trying to access object property that might not exist: returning non-maybe number is possible.

This becomes especially annoying with unnecessary-optional-chain linting. When trying to access a like o?.[id].a, linting rule claims that the optional chain is unnecessary - which it obviously is not: id might be anything, and object with that key might not exist.

If one wants to be sure that {a: number} in type Obj = {[id: string]: {a: number}}; exists, keys should be some finite set of strings like type Obj = {[id: 'key1' | 'key2']: {a: number}}; and it should be accessed with finite set of strings, like here.

[lll000111]

Already answered?

https://github.com/facebook/flow/issues/4303#issuecomment-428264098

We're unlikely to add such an exception right now. It's only a lint, so feel free to suppress or disable it if it makes sense for you. As with array access, it is the programmer's responsibility to ensure that they're looking up a valid key.

Since you answered there we now have two threads...

[villesau]

true, let's copy the answer here and keep the discussion here:

I think the lint is maybe the best part of the whole functionality :D It enforces consistency between checks and types: Otherwise the types are much more likely to rot if they don't match conditions. It revealed already after couple of weeks of optional chaining usage many types that didn't had maybe type when they should have had, and vice versa unnecessary checks. But if it does not work with {[id: string]: SomeType}, that makes it much more difficult to use.

I find array access very different. With array access it is easy to know what is the last available index and stay secure. With arbitrary key access, you don't have such privilege, and the linting rule prevents easy checks. With object on the other hand, you can define finite amount of possible keys if you want to make it more secure. {[id: string]: SomeType} defines "you might have any key", not "You have every key"

[hewsut]

I ran over this situation in our codebase a couple weeks ago and was quite surprised.

I'm unclear if this is considered a bug or a design decision but FWIW it makes a ton of sense to me and would love to see this change.

[EugeneZ]

I'm unclear if this is considered a bug or a design decision

It must be a design decision since it is documented in the docs.

As for why, my guess is that it's because in many cases, arrays and indexed objects are used in ways that are "safe" by the programmer and if Flow didn't make this assumption, it would make the code very annoying.

You can simulate this in your own code by searching for all indexers ([key: string] and [string] should be a good start) and making the value a union with void. For example if you have a type Foo = {[string]: string} turn it into type Foo = {[string]: string | void}. This makes Flow accurately model the way this object can actually be used. You can do the same with arrays: type Foo = Array<string> becomes type Foo = Array<string | void>. You will see the Flow errors start to pile up in your code and and you will quickly get tired of fixing the issues.

But I wish it was a configuration option or something. It bothers me a lot because it's such a gaping hole and leads to a lot of bugs in our code.