Support 0.14 pure components

[gaearon]

This is initial pass which seems to work together with react-proxy@react-0.14 with the following limitations:

• Turn off react-transform-catch-errors because it doesn't know what it's doing
• Top-level component must not be pure
• Changes in files that contain only pure components won't cause a redraw
• Some edge cases might be broken:
  • export arrow or function
  • module pattern?

We'll need to address each of those before merging this.

[joshblack]

What kind of strategy do you want to take for ExportDefaultDeclarations of ArrowFunctionExpressions or FunctionExpressions? Try to parse the file name? Or should we just focus on ExportNamedDeclarations?

[gaearon]

@joshblack

The default export name should probably be based on filename, the named exports can use their respective names.

[joshblack]

Sounds good, have some initial work for the following cases:

• Named exports for Function Declarations
• Named exports for Arrow Functions
• Default exports for an identifier to a Function Declaration
• Default exports for Arrow Functions (includes a naive approach for grabbing the file name as a name for the component)

Currently working on default exports for Function Expressions which might require a little more finesse with the Plugin visitors.

[gaearon]

It's not a big deal if we leave out some use cases at the beginning.
Just important that we don't break the code.

[gaearon]

I added you to collaborators so you can continue right on this branch.

[joshblack]

Sounds good! Pushed up the changes that I had along with a failing test case for the default export of a function expression.

[joshblack]

@gaearon

What's the module pattern that you wrote down as the edge case? I'm assuming you mean:

const ModulePattern = {
  NoProps() {
    return <div />;
  },
  WithProps(props) {
    return <div {...props} />;
  }
};

Which is similar to the fixtures you have already in react-proxy. And the expected transform:

var ModulePattern = {
  NoProps: _wrapComponent('_$NoProps')(function () {
    return _react2['default'].createElement('div', null);
  }),
  WithProps: _wrapComponent('_$WithProps')(function (props) {
    return _react2['default'].createElement('div', props);
  })
};

[gaearon]

By module pattern I mean this way of defining React components:

function Something() {
  return {
    render() { return <div />; } 
  }
}

At least it shouldn't break.

[gaearon]

We also need to make sure we don't wrap renderItem in

class List {
  render() {
    return <ul>{this.props.items.map(this.renderItem)}</ul>
  }
  renderItem(item) {
    return <ul>{item}</ul>; // using JSX, not called render(), shouldn't be wrapped
  }
}

[joshblack]

Oh okay, sounds good. What's the expected result for the module pattern? And for the second example, since React requires that classes extend Component should we just check to see if it's in a class that extends from that and is named render?

[gaearon]

What's the expected result for the module pattern?

It should be ignored and should not get wrapped (I don't plan to officially support it in transforms, but it shouldn't break either).

And for the second example, since React requires that classes extend Component should we just check to see if it's in a class that extends from that and is named render?

I don't want to check the base class because React 0.14 still allows non-Component descendants with a warning.

The important part is: false negatives (i.e. not wrapping something) are always better than false positives (wrapping the wrong thing). Are there any false negatives in the current solution?

[gaearon]

What I was trying to say regarding renderItem: it's not really about whether or not a method is called render(). My point was that we need to make sure we never interpret methods inside a class as pure components—regardless of whether they contain JSX. It's special-casing render name in code that worried me because individual methods must never be wrapped, regardless of their names.

[joshblack]

That makes total sense! And in regards to any false negatives, I'm going to try and add test cases that it should ignore to make sure. There's nothing I've added to suggest that it will provide false positives, but I definitely need to think more about the different ways people may use JSX in functions that could trigger the current implementation and create a false positive.

[bebraw]

Just a sidenote. If you can figure out a nice way to detect function based React components, that would open way for neater propType definitions. It would be cool to piggyback on Flow annotations without having to use Flow itself.

Consider the following:

const Viewer = (props: {json: Object, other: Object}) => {
  return (
    <div>
      <div>{'json: ' + JSON.stringify(props.json)}</div>
      <div>{'other: ' + JSON.stringify(props.other)}</div>
    </div>
  );
};

would transform to

const Viewer = (props) => {
  return (
    <div>
      <div>{'json: ' + JSON.stringify(props.json)}</div>
      <div>{'other: ' + JSON.stringify(props.other)}</div>
    </div>
  );
};

Viewer.propTypes = {
  json: React.PropTypes.object,
  other: React.PropTypes.object
};

This would make a fine Babel plugin of its own.

Anyway, keep up the good work on the PR.

[joshblack]

@bebraw cool! That sounds really interesting. Could also do something similar with default props if desired.

@gaearon

To try to align with the principle of favoring false negatives, I thought that a whitelist style approach might be more suitable. For example, for Arrow Function Expressions we could explicitly state a series of formatters based off of the type of the parent of the node. See: https://gist.github.com/joshblack/8fae4757474422ad632f#file-plugin-js with included actual and expected transforms to complete the POC.

This way, we could opt-in to specific transforms that we want to address and reduce the chance of collisions while making it easier to update existing implementations if a case isn't covered. Any thoughts?

Relevant AST Explorer Workspace

[bebraw]

@joshblack Exactly. I have a feeling it would be a nice way to introduce people to a small portion of Flow without forcing them to pick up new tooling.

[joshthecoder]

Any ETA when this will be ready? This will make it much nicer to use HMR in projects using pure components heavily. :)

How can I help with this issue? I would like to contribute to this but don't know where to start.
This issue is important to me and I would be glad to help you out @gaearon if I can.

[izaakschroeder]

☝️

[gaearon]

This is on my mind every single day, but I'm busy with other stuff I promised to ship before 0.14 happened. (Namely, Redux Egghead lessons.)
After I release the first batch of those, I'll have time to make this work. A few weeks please.

[joshthecoder]

@gaearon Thanks! If I can help in anyway let me know. Appreciate your hard work on these awesome libraries & tools.

[silvenon]

Asking for ETAs is not really showing appreciation. You can show it better by:

1. not polluting the comment section with 👍s and ETAs, instead just click on that nice "Subscribe" button to get notifications
2. actually helping instead of insincerely offering help (the roadmap is laid out in the first post, it's obvious what you can do to help)
3. giving money
4. sending a nice tweet

[joshthecoder]

@silvenon I would agree with you mostly except for 2. That's just 100% incorrect and I was completely sincere on my offer.

I have edited my original message to better reflect what I was trying to ask. Sorry if it came off as
a nagging question of "hey is this done yet". That was not my intention at all.

[Restuta]

@joshthecoder what steps have you already taken to help? If you will find predictable easy steps for noob in babel plugins please post'em here, so I won't repeat steps you've taken while figuring out where to start from.

[joshthecoder]

@Restuta Documentation does seem a bit light at the moment. So far I have just been picking up on it by reading existing plugins such as this one and also have dug around the babel-relay one.

[jamiebuilds]

Moving to #57 since this is out of date.

[ericclemmons]

For anyone else still following this, checkity check out issue #57 & PR #85.

[joeybaker]

Thanks @ericclemmons!

[gaearon]

You might be interested in reduxjs/redux#1455.

[ericclemmons]

We may still work something out. Not everyone has the option of moving all components & state to a single, re-renderable tree, myself included... :(

[gaearon]

React Hot Loader 3 supports functional components without destroying the state.
It supersedes both React Hot Loader and React Transform, and is built on lessons learned from both.
Check it out: gaearon/react-hot-boilerplate#61