Material-ui: Flow support is broken as of 0.57.0

[rosskevin]

PR #9311 adds this:

Warning

An existing bug in flow regarding the use of higher order components (HOC) limits the usefulness of flow by an application. As of November 27, 2017 we cannot at this time recommend the use of flow typing in your application in conjunction with material-ui until such time as a resolution is present.

For those getting started, I want to make sure and warn them to avoid any unnecessary effort if possible.

Expected Behavior

Applications should be able to use flow typing with material-ui.

Current Behavior

Due to https://github.com/mui-org/material-ui/pull/9311, the flow bug prevents use of proper HOC typing as would be used by an application.

Context

From https://github.com/mui-org/material-ui/issues/9109#issuecomment-347283291

We _cannot_ have proper flow support for apps since flow itself is broken. Since we cannot, not even an external libdef in flow-typed could have proper support. If we re-typed the HOCs to effectively any (as I discovered prior to #8419), you could pretend it had flow support, but we would have to do so knowing that we stopped static typing for almost everything. I found in #8419 with any HOCs, we were hiding 1700 flow errors! Essentially, we would have to make flow support almost useless.

Workaround

As a last resort, simply ignore material-ui in .flowconfig. If https://github.com/facebook/flow/issues/5382 remains unresolved for any amount of time, we will likely stop adding flow shadow files to the distribution, as it just makes every flow user ignore them. We aren't sure yet, but it's not a good situation.

Your Environment

| Tech | Version |
|--------------|---------|
| Material-UI | > v1.0.0-beta.21 |
| Flow | 0.57.0+ |

TL;DR

I went through considerable pain to get flow to work here and in our apps, only to have to abandon it. I have more invested with flow and material-ui than anyone, and I do think it says something important that I have chosen to abandon it for typescript. Now I think we should contemplate what this might mean for material-ui.

Next steps

TBD.

1. I will not be continuing to maintain flow in material-ui, at least as it pertains to external use.
2. @rsolomon did a good job with the latest PR to get us updated to 0.57+. He and others may be willing to step into the void I am leaving
3. HOCs in flow are in a very fragile state, and I found unusable external to material-ui due to https://github.com/mui-org/material-ui/pull/9311. I am actually surprised that flow is currently working inside material-ui because I could not get the same patterns to work in my app after the flow 0.57+ update.

I am open to transitioning the source files to Typescript, and moving flow outside the library to flow-typed (also see #7857). If the maintainers are not going to be using flow, I would prefer to follow this path.

I'm open to all discussion, I just wanted to make this situation known to all.

/cc @rsolomon @oliviertassinari @mbrookes @pelotom @sebald @m2mathew

[rosskevin]

I'm reposting a comment from @AriaMinaei here from https://github.com/digiaonline/react-flow-types/issues/20#issuecomment-345953452. I think it is relevant and well stated, and I now agree with his perspective.

This is off-topic, but FWIW, I've given up on flow. I've found myself spending way too much time wrestling with the type system, making sense of the cryptic errors, and having to do all of this every few days because I've either hit an edge case or a new version of flow has broken something.

Even though I haven't really measured how much time I spend on these issues precisely, I'm convinced that it's been a net loss of productivity for me.

I've written about it before on HN:

I advocated flow for two years, and I still find it interesting as a typesystem. But what finally led me to give up was the core team's lack of community engagement.

It's very difficult get the team's attention to a bug or a question. As a result, they fail to see the common pain points of actual users.

Typing higher order components is one common pain point that still doesn't have a clear solution. So, unless you're avoiding HOCs altogether, you can't really benefit from type annotations in your react codebase (you'll have too many implicit anys).

You'll find more of these common use cases that flow doesn't optimise for. They're all over the issues section. Unfortunately, issues on Github generally don't get much attention from the team either, so I'm guessing users sometimes give up on reporting them. This has happened to me quite a few times. I didn't report some bugs because putting together a reduced test case takes time, and that time would be wasted if your issue is not gonna be paid attention to.

What makes matters worse, is that there is no public roadmap. You don't know what the team's priorities are. You don't know when the bugs that affect your work are gonna get fixed, or some feature you need is gonna get implemented. You don't even know if it's on the radar.

Of course, the Flow team is under no obligation to do any of that. They have no obligation to fix bugs for us or publish a roadmap. I'm already grateful that they've given us access to their work and without charging money. However, they do have the responsibility to communicate what their priorities are. If they're positioning Flow as an alternative to TypeScript, which is a well-supported, community-oriented project, then they should state clearly that Flow simply isn't. Call it a research project, or a project focused on a single company's use cases. Don't ask people to bet their own projects on it, when they clearly shouldn't. It'll be a big loss of productivity for them down the line, and your messaging is partly responsible for that.
I'll happily keep reviewing and merging PRs on this repo, but personally, I've lost interest in flow.

[matthewdordal]

If this is the wrong place to ask, I apologize but how does one ignore material-ui in the .flowconfig? Just trying to figure out flow and so I added this in my .flowconfig

[ignore]
material-ui

Currently using material-ui v1.0.0-beta.22, and Flow v0.57.3

[rsolomon]

You can do this, which will effectively ignore the problems coming out of MUI while still allowing you to import Props from various components:

[ignore]
<PROJECT_ROOT>/node_modules/material-ui/node_modules/react-flow-types

[rsolomon]

@matthewdordal by the way, after upgrading to material-ui v1.0.0-beta.22 I was able to remove any ignores from my .flowconfig. I just had to switch to getting component Props using React.ElementProps<typeof (COMPONENT)> instead of importing the Props directly from the file. Ex:

import MUISwitch, { type Props } from 'material-ui/Switch/Switch';

...becomes...

import MUISwitch from 'material-ui/Switch/Switch';
type Props = React.ElementProps<typeof MUISwitch>;

[matthewdordal]

Thank you @rosskevin! I was having a hard time with

[ignore]
<PROJECT_ROOT>/node_modules/material-ui/node_modules/react-flow-types

But I was able to get this to work

<PROJECT_ROOT>/node_modules/material-ui/.*\.js\.flow

I think this will basically tell flow to ignore material-ui completely though

[rosskevin]

For watchers - we are actively dropping flow support.

With the broken state of flow, we cannot in good conscience distribute flow files here - we feel it gives the mistaken impression of coverage when we know well at this point that too many implicit anys due to poor hoc support.

Community flow-typed definition?

If you are interested in maintaining a flow-typed definition, please let the community know so perhaps you may join forces together. With our current sentiment regarding flow, we have decided we will not distribute any flow files from this hosted repository. It is probably best that you take a branch of what is current on v1-beta before #9453 is merged so that you may copy existing definitions out of source into a libdef.

[KevinGrandon]

Sad to see this, but totally understandable. Any idea how much work it would be to implement ...$Exact<Props> to fix flow support? Just wondering if we can release a forked version /w fixes until we get a flow-typed library.

[rosskevin]

@KevinGrandon - Exact is not the problem with flow. Flow is broken at the HOC level https://github.com/facebook/flow/issues/5382, and particularly displays critically different behavior across file imports https://github.com/facebook/flow/issues/3522.

So there aren't any fixes until flow itself is fixed. Also, to be clear, any flow-typed library produced will not improve the situation until flow itself is fixed.

[rsolomon]

If you're releasing a flow-typed definition, all you need to provide is the external type definitions of each component. You won't need to care about what the HoC does, and therefore won't run into any problems with $Diff and its weird behaviors across files.

[rosskevin]

^^^ but understand that static analysis will stop at the HOC (which will turn it into an implicit any). So it will be of limited value, but still some. I just want all caveats known before someone jumps in and thinks they can right that ship.

[rsolomon]

I don't think it will need to completely stop there, but I am sure that will not be perfect. I have lots of HoCs defined in flow-typed files that are working fine (react-router, react-redux, react-css-modules). The challenge will arise when trying to account for defaultProps.

[rsolomon]

To expand on that, people that want to use withStyles within their own app and have type safety with Flow will have to adjust their usage of defaultProps to gain the benefits. I would advise against the HigherOrderComponent flow def in that library-- the complexity and fragility will probably not be worth it to most consumers.

[prastut]

I am currently working on integrating MUI with a codebase that uses Flow for type checking. During my research on how to workaround Flow errors, I went through issue 9312 and PR 9453 (Not referencing them as don't want unnecessary bubbling up of Github thread). I completely understand why you would drop flow support.

I see that some of the flow maintainers did try to reach out here: but I guess it was too late by then.

But one thing I was not able to find in MUI's documentation for flow is the current state of how devs can integrate MUI with an existing codebase that uses flow. Our codebase uses other HOC libraries as well like recompose so I am not sure how best I can ignore flow errors from Material UI but let the rest of the code getting type checked. Moving to Typescript on our end would be a very drastic step, therefore some clarity would help.

I am more than happy to update the documentation if I have some clarity myself. Right now it looks as if the docs tell that we do support Flow but with a warning, but my experience reading all of the above links suggest that you have completely done away with Flow.

@rsolomon provides some information but it's lost in terms of getting context for a third person like me.

cc: @rosskevin

[rsolomon]

@prastut if you use the flow-typed libdefs for MUI, then Flow shouldn't be registering errors when the components are used correctly.

Can you share what version of Flow and Material-UI you are using, and if you are utilizing the flow-typed definitions linked above? Also, an example of the errors you are seeing with the associated usage would be helpful.

[rsolomon]

Also: Are you using withStyles? If so, the type definitions in flow-typed are out of date. I can spend some time updating them if that's the root of the errors you are seeing.

[prastut]

@rsolomon apologies for the delay in my reply. I

 "flow-bin": "^0.66.0",
 "flow-typed": "^2.3.0",
 "material-ui": "^1.0.0-beta.37",
 ```

I am using flow-type definations `material-ui_v1.x.x` .  An example of errors with associated usage: 

Error log: 

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ frog/imports/ui/TeacherView/OrchestrationView.jsx:44:15

Cannot create Grid element because in property children:
• Either React.Element [1] is incompatible with React.Portal [2].
• Or property @@iterator is missing in React.Element [1] but exists in $Iterable [3].

 frog/imports/ui/TeacherView/OrchestrationView.jsx
 40│               openActivities={session.openActivities}
 41│             />
 42│           ) : (
 43│             <Grid item xs={12}>

[1] 44│
45│
46│
47│
48│
49│
50│
51│
52│
53│
54│ )}
55│
56│ ) : (

 /private/tmp/flow/flowlib_39a09926/react.js

[2] 19│ | React$Portal
[3] 20│ | Iterable;

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ frog/imports/ui/TeacherView/OrchestrationView.jsx:44:16

Cannot create Card element because object type [1] is not a React component.

 frog/imports/ui/TeacherView/OrchestrationView.jsx
  41│             />
  42│           ) : (
  43│             <Grid item xs={12}>
  44│               <Card>
  45│                 <CardContent>
  46│                   <SessionInfo session={session} />
  47│                   <GraphView session={session} />
  48│                 </CardContent>
  49│                 <CardActions>
  50│                   <OrchestrationCtrlButtons session={session} />
  51│                 </CardActions>
  52│               </Card>
  53│             </Grid>
  54│           )}
  55│         </Grid>

 flow-typed/npm/material-ui_v1.x.x.js

[1] 223│ declare module.exports: {
224│ CardActions: $Exports<'material-ui/Card/CardActions'>,
225│ CardContent: $Exports<'material-ui/Card/CardContent'>,
226│ CardHeader: $Exports<'material-ui/Card/CardHeader'>,
227│ CardMedia: $Exports<'material-ui/Card/CardMedia'>,
228│ default: $Exports<'material-ui/Card/Card'>
229│ };

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ frog/imports/ui/TeacherView/OrchestrationView.jsx:16:8

Cannot instantiate React.Element because in type argument ElementType:
• Either a callable signature is missing in object type [1] but exists in
React.StatelessFunctionalComponent [2].
• Or object type [1] is incompatible with statics of React.Component [3].

 frog/imports/ui/TeacherView/OrchestrationView.jsx
  13│ const OrchestrationViewController = ({ classes }) => (
  14│   <div className={classes.root}>
  15│     <Grid item xs={12}>
  16│       <Card>
  17│         <CardContent>
  18│           <Typography>Word of the Day</Typography>
  19│         </CardContent>
  20│         <CardActions>
  21│           <Button size="small">Learn More</Button>
  22│         </CardActions>
  23│       </Card>
  24│     </Grid>
  25│   </div>
  26│ );

 flow-typed/npm/material-ui_v1.x.x.js

[1] 223│ declare module.exports: {
224│ CardActions: $Exports<'material-ui/Card/CardActions'>,
225│ CardContent: $Exports<'material-ui/Card/CardContent'>,
226│ CardHeader: $Exports<'material-ui/Card/CardHeader'>,
227│ CardMedia: $Exports<'material-ui/Card/CardMedia'>,
228│ default: $Exports<'material-ui/Card/Card'>
229│ };

 /private/tmp/flow/flowlib_39a09926/react.js

[2] 150│ | React$StatelessFunctionalComponent
[3] 151│ | Class>;

I am also using `withStyles` as well but that isn't giving me errors so far.  


I tried [MUI's flow example library](https://github.com/mui-org/material-ui/tree/v1-beta/examples/create-react-app-with-flow) but I think it's not working correctly. When I tried the following piece of code inside the repo it doesn't throw up errors: 

^ Why I think this should throw up error, because with flow-typed lib defs of MUI, it does: 

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ frog/imports/ui/TeacherView/OrchestrationView.jsx:43:28

Cannot create Grid element because in property xs:
• Either number [1] is incompatible with boolean [2].
• Or number [1] is incompatible with number literal 1 [3].
• Or number [1] is incompatible with number literal 2 [4].
• Or number [1] is incompatible with number literal 3 [5].
• Or number [1] is incompatible with number literal 4 [6].
• Or number [1] is incompatible with number literal 5 [7].
• Or number [1] is incompatible with number literal 6 [8].
• Or number [1] is incompatible with number literal 7 [9].
• Or number [1] is incompatible with number literal 8 [10].
• Or number [1] is incompatible with number literal 9 [11].
• Or number [1] is incompatible with number literal 10 [12].
• Or number [1] is incompatible with number literal 11 [13].
• Or number [1] is incompatible with number literal 12 [14].

  frog/imports/ui/TeacherView/OrchestrationView.jsx
   40│               openActivities={session.openActivities}
   41│             />
   42│           ) : (

[1] 43│
44│ )}
45│
46│ ) : (

  flow-typed/npm/material-ui_v1.x.x.js

[2] 614│ | boolean
[3] 615│ | 1
[4] 616│ | 2
[5] 617│ | 3
[6] 618│ | 4
[7] 619│ | 5
[8] 620│ | 6
[9] 621│ | 7
[10] 622│ | 8
[11] 623│ | 9
[12] 624│ | 10
[13] 625│ | 11
[14] 626│ | 12;

```

I am happy to help to update documentation/code and provide a more definite consensus on what the maintainers think about flow's usage.