Enable types-first mode in Flow #1539
Conversation
| const EditProfile = ({ | ||
| $c, | ||
| ...props | ||
| }: Props): React.Element<typeof UserAccountLayout> | null => { |
There was a problem hiding this comment.
React.MixedElement also works as a return type, as a generic way of saying it returns a React element without specifying which one. Though there is at least one case I saw where specifying it was useful.
| import SubHeader from './SubHeader'; | ||
|
|
||
| type Props = { | ||
| +editTab?: React.Node, | ||
| +editTab?: React.Element<EntityTabLink>, |
There was a problem hiding this comment.
Here's an example of where it might be useful to specify the type of element we want.
There was a problem hiding this comment.
<CatalystContext.Consumer> should be documented at https://github.com/metabrainz/musicbrainz-server/blob/master/HACKING.md#porting-tt-to-react.
Works for me otherwise. It’s unfortunate that automated changes (flow codemod, eslint) cannot be split off from manual changes in last commit, but I guess that would have broken code in between commits.
| +area: AreaT, | ||
| }; | ||
|
|
||
| const AreaContainmentLink = ({area}: Props) => ( |
There was a problem hiding this comment.
Wouldn't just {area}: {area: AreaT} be simpler here?
There was a problem hiding this comment.
It made the line too long once : Expand2ReactOutput | null was added in the last commit. I could've put the return type on a separate line, I guess, though I defaulted to making the Props type separate 'cause I think it makes adding new props a bit easer, and allows for reusing the type if we need to. But I don't mind either too much otherwise.
This will obsolete most uses of withCatalystContext.
https://flow.org/en/docs/lang/types-first/ 1. Types have been added to all exports to make them well-formed. 2. withCatalystContext has been removed, because it's a pain to type properly and isn't needed in most cases. The template renderer has been changed to always pass $c as a prop, and <CatalystContext.Consumer> can still be used directly in the cases where context is useful. withCatalystContext is a pain to type because its type parameter expects a props type with $c, but the export type of the file is without $c. So a well-formed export might look like: (withCatalystContext<PropsWithC>(Component): React.AbstractComponent<PropsWithoutC, void>); ...which is after separately defining types for PropsWithC and PropsWithoutC in the file. 3. Any Flow errors I came across while adding export types have been fixed. (It did reveal a couple cases where our types were completely wrong, though it doesn't appear these cases were related to issues at runtime.)
|
Documented the |
Problem
Flow is making https://flow.org/en/docs/lang/types-first/ the default architecture and removing the "classic" mode by January, so we have to prepare our codebase for that.
Solution
Adds type signatures to all of our exports to make them "well-formed." Then enables
types_firstin .flowconfig.This requires changing a lot of lines, to say the least.