Flow: Support ES7 class property initializers

[bolinfest]

This would make it easier to follow the new ES6+ pattern for declaring React classes as described in http://babeljs.io/blog/2015/06/07/react-on-es6-plus/. In particular, this is needed for declaring static propTypes.

[samwgoldman]

We're lucky to have @jeffmo, who is not only a Flow maintainer, but also the champion of this proposal to TC39.

My understanding is that we're not going to move on this until the proposal has made some more progress (which is in progress, presumably as I type), but I'll defer to Jeff.

[bolinfest]

True, but our collective co-workers at FB would really like to write React in the style that I linked to today :) I'm optimistic.

On the bright side, as of Flow 0.16.0, it is at least possible to use this feature now and suppress the error from Flow, so the parser knows about this syntax, it just kinda ignores it.

Our .flowconfig:

[options]
suppress_comment=.*\\$FlowIssue.*

A React component:

class FileTreePanel extends React.Component {
  // $FlowIssue https://github.com/facebook/flow/issues/850
  static propTypes = {
    initialWidth: PropTypes.number,
    onChangeExperimentalHgIntegration: PropTypes.func.isRequired,
    store: PropTypes.instanceOf(FileTreeStore).isRequired,
  };

  render() {
    return (
      <div className="nuclide-file-tree-deux-container">
        <PanelComponent
          dock="left"
          initialLength={this.props.initialWidth}
          ref="panel">
          <FileTree store={this.props.store} />
        </PanelComponent>

        <div className="nuclide-file-tree-deux-footer">
          <NuclideCheckbox
            checked={atom.config.get('nuclide-file-tree-deux.enableExperimentalVcsIntegration')}
            onChange={this.props.onChangeExperimentalHgIntegration}
            >
            &nbsp;Use experimental Hg integration
          </NuclideCheckbox>
        </div>
      </div>
    );
  }

  getFileTree(): FileTree {
    return this.refs['panel'].getChildComponent();
  }

  getLength(): number {
    return this.refs['panel'].getLength();
  }
}

[LegNeato]

Some outside Facebook would like this too :wink:

[LegNeato]

Also unfortunate that the workaround requires adding an extra ; but I'll take it!

[bolinfest]

Currently, it's possible to have Flow suppress all diagnostics about decorators by specifying the following in .flowconfig:

[options]
esproposal.decorators=ignore

I would expect a similar option for this feature while it's still in stage 1.

[jeffmo]

Class properties made it to stage 1 this week, which means TC39 agrees this is a proposal to work on and that the direction is good. Static property initializers are uncontroversial so I intend to build this into Flow. I have a few other things to finish up first, but otherwise this is near the top of my priority list...so I'd say starting in on it next week is a likely timeframe.

There's some level of controversy around the syntax of instance properties (basically whether the declarations should sit in the constructor or if it's ok for them to sit where they are in the current proposal) and still a lot of work to do to drive them to a more stable proposal stage. As a result, I'll probably move forward with the proposal syntax as-is under a separate config option -- but usage of that will have to be considered a little "unstable" for at least a while as we will track any progress/updates to the spec as they happen which may or may not be backward compatible.

[idolize]

@bolinfest This suppression option does not appear to work with decorators on _classes_ (only on _methods_).

I opened a PR on one of the tests for this feature to illustrate the bug/missing feature: https://github.com/facebook/flow/pull/1001

[niieani]

:+1: esproposal.decorators=ignore does not work for decorated classes, would be great to get this fixed.

[jeffmo]

Just following up a bit (better late than never, I suppose):

We now have two new config options: esproposal.class_static_fields and esproposal.class_instance_fields. Setting either or both of these to enable should allow Flow to understand field behavior as spec'd in the latest version of the proposal.

[STRML]

@jeffmo Is setting class_static_fields to enable supposed to make Flow understand propTypes? I am still not able to find a solution for ES6 React classes using propTypes. I could convert propTypes over to a Flow typedef, but I enjoy the runtime checks.

[samwgoldman]

Flow understands PropTypes by hardcoding support for React.createClass during AST traversal. It's super effective, but we are working to remove that kind of hard-coded support moving forward, so it's very unlikely we will add support for PropTypes to ES6 classes.

I think a better approach would be a Babel plugin that converts Flow type annotations to propTypes. I don't know if anyone is working on that, but I just want to champion that idea.

[STRML]

@samwgoldman That's a good idea - could be a fun project. Thanks for the update.

[brigand]

I started working on an implementation of that idea: babel-plugin-flow-react-proptypes.

[STRML]

Great project! This will mean we can finally move off proptypes in our app.
Thanks so much for taking this on.
On Mar 16, 2016 1:01 AM, "Frankie Bagnardi" [email protected]
wrote:

I started working on an implementation of that idea:
babel-plugin-flow-react-proptypes
https://github.com/brigand/babel-plugin-flow-react-proptypes.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://github.com/facebook/flow/issues/850#issuecomment-197165934

[samwgoldman]

:dancers:

Also going to close this, since we do actually support the proposed syntax behind a flag, per Jeff's comment.