Support pure components

[gaearon]

We should extend the transform to find and wrap pure components.
I think a good enough heuristic for us is to catch functions that contain JSX.

We should also support them in react-proxy: gaearon/react-proxy#12

[joshblack]

Would love to help tackle this issue here and in react-proxy, any advice on getting started?

[gaearon]

@joshblack

I just pushed pure branch, it contains some initial work.
Currently it fails on fat arrows as I'm not sure how to extract identifier from their assignment.
There's a failing test which should help.

[joshblack]

Why is the expected output of the pure test with the Arrow Expressions B and C:

function B() {
  function C() {
    return _react2['default'].createElement('div', null);
  }
}

Is that the correct behavior versus something like:

var B = (function B() {
  return _react2['default'].createElement('div', null);
},  = _wrapComponent('_$B')(B));
var C = (function C() {
  return _react2['default'].createElement('div', null);
},  = _wrapComponent('_$C')(C));

With _components having the value:

var _components = {
  _$A: {
    displayName: 'A',
    isFunction: true
  },
  _$B: {
    displayName: 'B',
    isFunction: true
  },
  _$C: {
    displayName: 'C',
    isFunction: true
  }
};

[gaearon]

You're right about _components.
As for the previous remark, there's

A = _wrapComponent('_$A')(A)

right afterwards. But whatever else that does the job will work—as long as it keeps semantics and binding name.

[gaearon]

Oh, sorry, I see what you mean now. This is some outdated version of the test so please disregard this part of it. I seem to not have finished the test :-(

[joshblack]

No problem! Was just checking. For the expected output then, would it make sense for it to be

// actual
let B = () => <div />;
const C = () => <div />;

// expected
var B = _wrapComponent('_$B')(function () {
  return _react2['default'].createElement('div', null);
});

var C = _wrapComponent('_$C')(function () {
  return _react2['default'].createElement('div', null);
});

or does this pattern change because it's an Arrow Function? I'll update the _components part in the test as well to reflect _$B and _$C. To get these I think the easiest way is to just pass parent down into findDisplayName and just check if it's an Arrow Function and Variable Declarator. If it is, the display name will just be parent.id.name. For example:

  function findDisplayName(node, parent) {
    if (node.id) {
      return node.id.name;
    }
    if (t.isArrowFunctionExpression(node) && t.isVariableDeclarator(parent)) {
      return parent.id.name;
    }
    if (!node.arguments) {
      return;
    }
    const props = node.arguments[0].properties;
    for (let i = 0; i < props.length; i++) {
      const prop = props[i];
      const key = t.toComputedKey(prop);
      if (t.isLiteral(key, { value: 'displayName' })) {
        return prop.value.value;
      }
    }
  }

[joshblack]

Have a rough initial pass at this that passes an updated test here: joshblack@0f9a777.

Had to figure out an alternative to the existing pattern for a Function Declaration (A = _wrapComponent('_$A')(A)) because of a dynamic node internal error that occurs when trying to apply that call expression pattern to Arrow Functions. Instead, I just create a call expression that passes in the value of the node as an argument. For example:

var B = _wrapComponent('_$B')(function () {
  return _react2['default'].createElement('div', null);
});

[christianalfoni]

Hi guys,

I got the time to look into this, but it is a lot for me to get my head around. Is this repo the only part of the solution? I have seen references to react-proxy and creating a new transformer to support this? Is that the case, or will it work if this plugin is able to identify the functions?

I tried your fix @joshblack. Using:

function App() {
  return (
    <div>happ</div>
  );
}

export default App;

But it does not actually do the rerender on changes. It also failed with:

export default function App() {
  return (
    <div>happ</div>
  );
}

But thanks for the discussion here, it helped me getting a sense of the challenge :-)

I would like to take a crack at it, but yeah, I just have to know if this is where to fix it

[christianalfoni]

Would also be nice to have some pointers to what actually does the rerendering of the component. As I understand most of the code is related to identifying and replacing syntax? Where does it actually make the component render itself again? :-)

[joshblack]

My understanding is that the point of this project is to codemod application code and provide a wrapper around all supported components. This wrapper is where react-proxy comes into play, which enables HMR to occur when developing.

[christianalfoni]

Aha, I see now. So in theory your fix probably works? It is just react-proxy that needs to understand how to handle a simple function as a stateless component?

[gaearon]

If you'd like to try, I pushed react-0.14 branch to react-proxy. Not sure it works but feel free to experiment!

[christianalfoni]

Hm, it did not seem to work. But I am hacking this together, so might have done something wrong. Any pro tips on setting up an environment where all of this can be tested at the same time? :-)

[gaearon]

Maybe it doesn't. ;-) You can check whether react-0.14 branch tests pass; if they do, you can try looking what's different in code generated by Babel plugin.

[gaearon]

@christianalfoni

Just to make it clear—even with this change, it won't work if the top-level component is pure. AFAIK there's no easy way we can make it work for top-level component if it is pure.

[christianalfoni]

Ah, I see, that might have been it. Let me do a new check ASAP. It has been difficult helping out with this. I need to learn by changing the code and see the end result... its just how I learn, hehe. Not quite sure how to set up all the dependencies in a way that lets me change each part and see the end result :-) But I am happy to verify code, so I will get back to you!

[gaearon]

You can install react-transform-boilerplate, react-proxy, babel-plugin-react-transform locally. Then run

# assuming [email protected] with flat dependencies

cd react-transform-boilerplate
npm link ../react-proxy
npm link ../babel-plugin-react-transform

Any time you change either of their code, you can run npm link again to recompile it.

[gaearon]

That said I already discovered a problem with that commit so maybe you can test later. :-)

[christianalfoni]

Hehe, cool! Let me know @gaearon :-)

[gaearon]

Superseded by #34.