[meta] Bringing Flow types in line with 16.3 APIs

[billyjanitsch]

Hi!

I realize that the React team doesn't work on Flow directly, and that the React typings live there. However, there are currently some large gaps in Flow support for the new 16.2/16.3 APIs, and I thought it might be good to raise/track them here, for the benefit of people searching the React tracker.

I filed issues in the Flow repo for missing typings that didn't have outstanding PRs.

• UNSAFE_-prefixed lifecycles (commit: facebook/flow@cd8fdcd)
• createContext (commit: facebook/flow@dd27ebb)
• createRef (commits: facebook/flow@dd27ebb, facebook/flow@d74ab13)
• componentDidCatch (PR: Add componentDidCatch React lifecycle flow#6044)
• forwardRef (PR: Add definition for React.forwardRef() flow#6510)
• getDerivedStateFromProps (PR: Add React lifecycles getDerivedStateFromProps, getSnapshotBeforeUpdate flow#6511)
• getSnapshotBeforeUpdate (PR: Add React lifecycles getDerivedStateFromProps, getSnapshotBeforeUpdate flow#6511)
• StrictMode (PR: Add React.StrictMode flow#6078)

Feel free to close this issue if you don't think it's useful. But maybe the React team knows someone on the Flow team who can help give priority to these libdef updates (especially those with PRs)?

❤️

[Jessidhia]

I am curious to see how/if Flow will deal with getDerivedStateFromProps. It's not currently possible to safely type it in TypeScript's definition because static methods are not allowed to reference instance generic parameters.

class Component<P, S> {
  // this is an error because P/S are per-instance types from the POV of the compiler
  static getDerivedStateFromProps (props: P, nextState: S): Partial<S>|null
}

[gaearon]

In Flow, it's defined as

  static getDerivedStateFromProps(
    nextProps: Props,
    prevState: State
  ): $Shape<State> | null {
    // ...
  }

[mrkev]

Three of the ones missing are issues without a PR. Is there anything blocking a PR? If not, feel free to make one and ping me to try and merge 👍

[motiz88]

@mrkev I've just opened facebook/flow#6510 to close facebook/flow#6103 (forwardRef).

[motiz88]

Also opened facebook/flow#6511 to close facebook/flow#6101 (getDerivedStateFromProps) and facebook/flow#6104 (getSnapshotBeforeUpdate).

[rickyp-uber]

Are there any goals for the React library to own it's own definition types (e.g. move the definitions into this project)?
Question requested here: facebook/flow#5474 (comment)

[gaearon]

My understanding is that in many cases typing changes to React also require some changes to Flow source (due to how React features are modeled) so I'm not sure it would actually fix anything except adding more friction to deploying these changes.

[rickyp-uber]

Agreed about the current state of the definition but to answer this specific question let's assume we can write the entire React flow definition in a *.js.flow file... would the React team be willing to host this in their own code base or do they want the Flow team to own it in their code base?

[gaearon]

How would release cycle work?

[rickyp-uber]

How do you mean? Since the *.js.flow file is within the React library, it gets released whenever the npm package is published. It's on the React team to make sure the definitions defined in the *.js.flow file are inline with the package's contents.

From a consumer standpoint, Flow reads the *.js.flow in the npm package installed and get's the types. If the npm package's API changes, the *.js.flow within that package should reflect it so when the consumer updates to the newer version, Flow should yell about the break. This actually ends up being easier and safer on the consumer because if they are using an older version of React, say 16, and flow-bin has definitions for React 18, the static type definitions may not be correct and could result in runtime failures.

Annoyingly, Flow doesn't have this documented on their website currently and clicking the link shoots off an alert that says "TODO". Overall though this is a similar design as typescript's definition files where for typescript's case instead of having an extra file at the root of the project, they have the definition location defined in the package.json.

[gaearon]

If the npm package's API changes, the *.js.flow within that package should reflect it so when the consumer updates to the newer version, Flow should yell about the break.

What if Flow itself makes a breaking change? Is the expectation that .flow.js that ships React is only guaranteed to work with latest Flow? Or that it's backwards compatible even if that results in a worse definition?

[rickyp-uber]

I think in general it's that the *.flow.js would generally only work with the latest version of flow. It would be on the consumer when they update the package to also update their version of flow if flow does fire an error.

For supporting multiple flow versions, you'd have to ask the Flow team but I assume that's not a goal currently.

[EdwardDrapkin]

I think in general it's that the *.flow.js would generally only work with the latest version of flow. It would be on the consumer when they update the package to also update their version of flow if flow does fire an error.

The issue with this here is downstream React projects that also use Flow, e.g. React-Native. RN usually lags a few versions behind Flow (as do most projects that use Flow IME) and if you pin React to the latest version of Flow, then you prevent downstream React projects from updating to the latest React version until they update to the latest version of Flow. Some of the Flow releases require significant effort to upgrade because of changes (e.g. version 68 changed the way Flow interacted with unknown properties on objects and required a heavy refactor in some code). I don't think anything that makes upgrading React more difficult is a good idea unless the benefit that it presents is exceedingly obvious, and I'm still not sure what the actual benefit of moving the libdef would be.

[jbrown215]

forwardRef landed in facebook/flow@9a3377a

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[mrkev]

bump

[stale]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[stale]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!