Material-ui: react-docgen issues related to flow upgrade

[rosskevin]

A couple of issues found with recent flow changes, documented by @oliviertassinari and consolidated here.

I'll be working these issues into PR #9203

• [x] [react-docgen doesn't mark defaulted properties as optional](https://github.com/reactjs/react-docgen/issues/221) (PR)
• [x] classes missing in some components (needed for docs, not sure if flow needs it)
• [x] theme added to components wrapped in withStyles
• [x] casts inside defaultProps unexpectedly show in docs

---

From: https://github.com/callemall/material-ui/pull/9212#issuecomment-345435325

@rosskevin Some broken things I have noticed introduced with #8983. I don't think that we should put flow interest on top of API ones. Way more people read the API documentation than uses flow.

• We mark properties required while they are not.

-| component | ElementType | 'div' | The component used for the root node. Either a string to use a DOM element or a component. |
+| <span style="color: #31a148">component *</span> | ElementType | 'div' | The component used for the root node. Either a string to use a DOM element or a component. |

• The classes property is missing in some components.

-| classes | Object |  | Useful to extend the style applied to components. |
+| classes | Object |  |  |

• A theme property has been added to all the components.

+| theme | Object |  |  |

From: https://github.com/callemall/material-ui/pull/9203#issuecomment-345377494

A sample of the API diff it's introducing:

 | Name | Type | Default | Description |
 |:-----|:-----|:--------|:------------|
 | classes | Object |  | Useful to extend the style applied to components. |
-| <span style="color: #31a148">component *</span> | ElementType | 'div' | The component used for the root node. Either a string to use a DOM element or a component. |
+| <span style="color: #31a148">component *</span> | ElementType | 'div': ElementType | The component used for the root node. Either a string to use a DOM element or a component. |

I'm wondering if we can avoid 'div': ElementType. Any idea?

[rosskevin]

@oliviertassinari regarding theme - we have a choice:
a) remove optional injected theme from withStyles (and use withTheme instead); or
b) every component inside withStyles must have theme

These are based on my current understanding. I'll dig in and make sure.

@rsolomon I'm curious as to why we need theme but can omit classes. I'm going to dig in now but any notes are helpful.

[rosskevin]

Note: The current react-docgen PR https://github.com/reactjs/react-docgen/pull/227 is based on master, not the 3.0-dev we depend on.

[rsolomon]

@rosskevin removing either classes or theme both cause issues, but the issue is that the component becomes untyped instead of throwing errors. I think it's because of this case: https://github.com/digiaonline/react-flow-types/blob/develop/index.js.flow#L38

Unfortunately that cases serves as an important fallback for untyped components, such as all of the demo components. I can't think of a better alternative off the top of my head, unless we'd be OK with ignoring those directories.

[rsolomon]

This is news to me, by the way. If I would have realized that change made some errors potentially get swallowed I would have tried working through a solution. I'm thinking about what to do right now, but nothing is immediately coming to mind.

[rsolomon]

Now that I think about it, I suspect I didn't notice this issue because of the casts to ComponentWithDefaultProps.

[oliviertassinari]

regarding theme - we have a choice:

@rosskevin I see another solution. We can systematically ignore this property when generating the documentation.

[rsolomon]

I think L36 and L38 can be safely removed from this file in react-flow-types. It doesn't fix the issue of requiring theme, but it does fix the issue of swallowing errors. However...

We can systematically ignore this property when generating the documentation.

That's the best thing I can think of as well.

[rsolomon]

Let me know if I can help with any of these potential tasks.

[rosskevin]

I just submitted react-docgen PR https://github.com/reactjs/react-docgen/pull/231

It solves required/optional and flow type casts.

[rosskevin]

@rsolomon if you can PR the change to react-flow-types that would be most helpful. I've almost got all the docs changes here wrapped up. Still one bug on my docgen PR for a good pass on material-ui codebase.

[rsolomon]

PR up here: https://github.com/digiaonline/react-flow-types/pull/19