React: Include flow type definitions

[jamiehodge]

As per comments from @samwgoldman in the following issue, the flow built-in react type definitions are lagging behind and the recommendation is that the definitions be included and maintained in the react repo itself:

https://github.com/facebook/flow/issues/1081

The flow maintainers are doing an excellent job of implementing new JS syntax/features. It would be great if they did not also need to maintain this file:

https://github.com/facebook/flow/blob/master/lib/react.js

One could, of course, also relegate this definition to yet another repo.

[jimfb]

Flow is open source, so if you find that something is wrong or out of date, I'm sure they would appriciate a PR to fix it - feel free to submit one to the Flow repository!

My intuition is that this file should remain in the flow/lib repository/directory (at least for now), along with all the other flow libs. Changing the location of the file won't fundamentally change anything - maintaining the file will still require exactly the same amount of work. We ran into a nearly identical issue with typescript in https://github.com/facebook/react/pull/5454, ultimately concluding that a request for such a change would need to come directly from the project maintainers, along with appropriate update and testing infrastructure as per https://github.com/facebook/react/pull/5454#issuecomment-155982256

[samwgoldman]

Thanks @jimfb, and sorry if my comment came off as an "official" position. I definitely don't have insight into the factors which inform these decisions. There are definitely some things we can do on the Flow side to make versioning less painful—off the top of my head, a more granular flowlib with control over imports, which would help for node/browser split as well.

[zpao]

I think ultimately this one does belong in the React repo. We've gone back and forth on this for a while and the big difference from TypeScript is that the primary contributors (FB) are using Flow extensively. There's a far greater chance we'll know when we do something that's not compatible with the typedefs. The better move for Flow long term is not to have any libraries included in its core (node, dom, etc are fine because they aren't libraries). I'm going to reopen since the whole Flow issue here is definitely an issue cutting across multiple projects and cc @sebmarkbage since he's been thinking about this a bunch lately.

[sebmarkbage]

For context, most of the Flow team sits one desk over from the React team at Facebook and both report to the same resource allocation which is seamless. The repo isn't what will make or break this collaboration.

The Flow team build and prioritize features based on the requirements of the React API or other core frameworks so it's a feature that they're able to maintain it and add new features as they deploy. They have a lot more context on how this can be typed and configured than we have.

We're currently looking at how Flow types can be deployed in npm packages. At that point it might make more sense that the React definitions are deployed with a React release.

Ultimately this isn't about maintainability but about how deploys and releases are made.

[jamiehodge]

Thank you for your comments. It seems as though there's plenty of thought being given to the issue. In the meantime, is there not a more up-to-date type definition file for 0.14+ react in use at FB? If so, would you mind sharing it?

[eikeon]

I'm assuming https://github.com/facebook/flow/blob/master/lib/react.js is still the place to be picking up the React declarations?

We're currently looking at how Flow types can be deployed in npm packages. At that point it might make more sense that the React definitions are deployed with a React release.

Is there anyway to pick up on the above React declarations or should we just be copying them into our projects for now? /cc @sebmarkbage

[eikeon]

In particular, how should the facebook/flux examples using flow be picking them up, for now. See: facebook/flux#336

[iamdustan]

I’d like to propose three possible paths forward here. Now that flow will automatically read {file}.js.flow files for definitions we could:

• ensure flow gen-flow-files .... works and generates all of the *.js.flow files in packages
• write a custom step to the build tooling to include the public renderer type def for each public package + a minimal typedef for ReactFiberReconciler.js in each package that receives that file. (everything else is considered internal)
• move the react defs to https://github.com/flowtype/flow-typed

Pros/cons:

The first two options have the benefit of “just working” for consumers. install React from npm and you automatically get the defs and flow will automatically read them.

The cons between the two are either depending on an unstable feature of flow (gen-flow-files) or maintaining a custom solution.

Additionally, the react def being maintained by flow has the benefit that the definition is able to take advantage of unstable Flow features and allow the flow team some more flexibility, though loses the ability to track React versions a bit (e.g. stateless functional components or children). React being maintained and distributed with the react source has the downside that breaking changes to flow releases may cause undesirable errors for consumers as they update one or the other.

The flow-typed approach that the typedefs can be written against specific versions of React and specific versions of flow. (e.g. [email protected] and [email protected]), at the cost of losing the out-of-the-box experience. Consumers would need to use the flow-typed CLI tool to install the react definitions for whatever version of React and Flow they are using.

[gaearon]

I don’t see this happening any time soon. The way React typings are implemented in Flow have more to do with Flow than with React. In practice keeping the typedefs located here won’t change this constraint, but will make it harder for Flow folks to iterate on them.

If someone from the Flow team is willing to own this and help us with this, that would be awesome. But I’m not aware of anything similar being a priority so I’ll just close this.