Support Flow typing in component_loader

[mjclawar]

Description

Right now, creating a custom Dash component requires using the prop-types library. react-docgen used in dash-components-archetype here does support Flow types, but the metadata.json parser in component_loader uses the fields generated from the PropTypes attribute of the component. Development of new components would be a potentially better environment if it also supported using Flow (see, e.g., a material-ui Dialog component. It would also allow faster porting of React components already written with Flow to Dash components.

Necessary pieces

• The eslint file in dash-components-archetype will need to use eslint-plugin-flowtype
• The babelrc file in dash-components-archetype will need to use transform-class-properties in order to support static defaultProps = {} (not required, but would be nice), and the react preset already handles stripping Flow types.
• package.json in dash-components-archetype would need (at least) eslint-plugin-flowtype and babel-plugin-transform-class-properties
• One of the following two to address the metadata:
  • metadata.json will need to be generated using the generated Flow types (matching the current structure) in dash-components-archetype, if they exist or
  • component_loader will need to support Flow types if they are found in the metadata.json and the fields generated by PropTypes are not found.

Note that some of the code changes would occur in dash-components-archetype, but figured here might be a better place to discuss since the component_loader is the biggest "Dash user"-facing piece. There may also be other necessary pieces that I missed.

We've set this structure up in the material-ui ports that we're using internally for our own material Dash components, and very much like it (we're also using jest for testing and would love to get the Dash toolchain updated to use React 16 since the current chain is tied to an older version of React).

If this makes sense, I can create issues specifically on dash-components-archetype and plan to get this out in the next couple of weeks. There should not be any breaking changes anywhere from addressing this issue.

Finally: insert flamewar about type hinting here 😄

[chriddyp]

There should not be any breaking changes anywhere from addressing this issue.

If so, then this sounds good to me! I totally respect flow users and the Dash component archetype should be as flexible as possible.

Could you point me to some code snippets in your material-ui project that shows how this currently works?

[chriddyp]

Would any of these additions to eslint, babelrc, or package.json affect component developers who are not using Flow? If they do affect those developers, then perhaps the setup could ask the developers if they would like to use flow or not:

builder-init dash-components-archetype
dash-components-archetype-0.2.11.tgz
[builder-init] Preparing templates for: dash-components-archetype
? Please name your component suite.
? Are you using Flow? (n/Y, default: n)

[mjclawar]

Could you point me to some code snippets in your material-ui project that shows how this currently works?
...
Would any of these additions to eslint, babelrc, or package.json affect component developers who are not using Flow?

There's actually not that much more since the babel react preset already supports stripping Flow types. But here's a FlatButton component that uses both Flow types and the prop-types package, since that is still needed for the metadata.json generation and parsing by Dash. The use of Flow there is just superfluous for paranoid typing.

The user could actually set up their own eslint to extend the current dash-components-archetype eslint files, but add in plugin:flowtype/recommended. For example, we completely replaced the dash-components-archetype eslint and babelrc in the sd-material-ui project without having any issues, and then just modified the build-dist commands to use a custom webpack config (which is definitely not necessary to get Flow set up). This now seems like a completely separate issue, and perhaps meant more for the documentation (e.g., a "How to add Flow" section on the Dash docs) than actually changing code.

Some related discussion about config via builder may be here plotly/dash-components-archetype#37 (comment)

Modified next steps

It does seem like after some further thought and your questions that the only thing really needed to support Flow out of the box is to modify the component_loader to handle the metadata.json types generated by the prop-types library if those exist, and otherwise look for the types in metadata.json generated by Flow (which have a different structure). Then a user could just write Flow-typed React code and not see any difference in the generated Dash Python component. This approach also would not modify existing behavior. However, this does have a couple of drawbacks:

1. A user making a dash component with both prop-types and Flow hinting for the props would generate a larger metadata.json file than necessary.
   • This would be a rare case since Flow can replace prop-types, and the benefit of prop-types for runtime checking is gone in the production build anyway
2. Potentially larger burden on the Python end of things to make decisions when parsing metadata.json
   • This seems fine as a very minor "compile" time hit.

If it makes sense, I or someone on my team can likely put something together for a component_loader that supports Flow-generated metadata.json next week.

[chriddyp]

If it makes sense, I or someone on my team can likely put something together for a component_loader that supports Flow-generated metadata.json next week.

I think that this makes sense and it seems like much lesser impact. Looking forward to seeing what this looks like!