Flow: use implicit exact object types

[mrtnzlml]

Flow standard is now to use { } for strict objects and { key: value, ... } for open objects, see: facebook/relay@6fa0b0d and facebook/react-native@00cfb0f

This change should be without any problems as longs as users of our NPM libraries are using Flow with exact_by_default=true (which is now pretty standard I'd say).

Here is how object types behave before this change:

type A = { x: number }; // already "exact" type but disallowed because it would be confusing to mix different syntaxes
type B = { x: number, ... }; // this is inexact
type C = {| x: number |}; // this is explicitly exact and the only allowed way how to describe type "exactness"

Here is how object types behave after this change:

type A = { x: number }; // this is the only allowed syntax for "exact" type
type B = { x: number, ... }; // this is still inexact
type C = {| x: number |}; // this is also exact but no longer allowed (so it's not confusing)

Some related (non-blocking) issues:

• Rule 'require-readonly-react-props' not compatible with implicit exact objects gajus/eslint-plugin-flowtype#467
• Missing flowlint for the old exact object type syntax facebook/flow#8612

TODO:

• backward compatibility: what about exported NPM libs with these new types
• coordinate with next major Eslint Config release: Eslint config: prepare next major release #2159

[mrtnzlml]

Draft only for now: still investigating what gets broken, what are the consequences, and how to make sure we keep the codebase unified.

[mrtnzlml]

@adeira/devs could you please have a look at this PR draft and share your opinion/concerns? Thanks! 😎

[michalsanger]

I'd say let’s do this.

think about backward compatibility

What exactly do you mean by this? BC within the Universe monorepo? Can it be achieved and do we even need it?

what about exported NPM libs with these new types?

Could a new major release be the solution?

[mrtnzlml]

think about backward compatibility

What exactly do you mean by this? BC within the Universe monorepo? Can it be achieved and do we even need it?

For example, how does it behave when we publish an NPM package with this new setup and users of our package are using the old setup? I would like to know the answer before giving this PR for a proper review.

I guess both of these TODOs are the same and I had the question twice without noticing. 😬

what about exported NPM libs with these new types?

Could a new major release be the solution?

It can be a solution, yes. I'd like to check first if it's necessary though. Also, Flow usage is quite low so we probably can cut some corners.

[michalsanger]

If I understand this correctly, when you are using the old syntax and start mixing it with the new one, you are basically using weaker types. You don't get any Flow errors. In this meaning it's backward compatible, but from DX perspective I would appreciate a big info in changelog. Major version could be that big info, on the other hand do you see any libs with Flow types doing this?

[mrtnzlml]

If I understand this correctly, when you are using the old syntax and start mixing it with the new one, you are basically using weaker types. You don't get any Flow errors. In this meaning it's backward compatible, but from DX perspective I would appreciate a big info in changelog. Major version could be that big info, on the other hand do you see any libs with Flow types doing this?

Edit: yeah, that's pretty much correct (I missed the "when you are using the old syntax" part). If you are still using exact_by_default=false then you are out of luck.

I believe this is not correct. There are many changes in this PR but they are mostly syntactical - no big changes in terms of behavior. That's thanks to the fact that we are using "exact by default" behavior for a long time. Here is how it is now:

export type A = { x: number }; // this is already "exact" type but disallowed because it would be confusing to mix different syntaxes
export type B = { x: number, ... }; // this is inexact
export type C = {| x: number |}; // this is explicitly exact and the only allowed way how to describe type "exactness"

Here is what this PR changes:

export type A = { x: number }; // this is the only allowed syntax for "exact" type
export type B = { x: number, ... }; // this is still inexact
export type C = {| x: number |}; // this is also exact but no longer allowed (so it's not confusing)

That's why I believe that changing {| x: number |} to { x: number } in our NPM libraries should be fine (with no BC break and no change in the type soundness) as long as you use exact_by_default=true. This feature was rolled out as follows:

• v0.101.0 (10 Jun 2019) new implicit-inexact-object lint
• v0.105.0 (9 Aug 2019) new exact_by_default option in flowconfig
• we started using exact_by_default=true 9 Nov 2019, see: a9037e6
• v0.112.0 (12 Nov 2019) new ambiguous-object-type lint
• v0.148.0 is the latest Flow version

This timeline makes me think that it should be fine to proceed (6 Apr 2021). Please note though that I didn't actually try it yet so I might be wrong.

[mrtnzlml]

OSS versions of FBT and Recoil are now also getting this change, see: https://github.com/search?q=%22fb-www%2Fflow-exact-by-default-object-types%22&type=commits (fbt, recoil)