(WIP) Separate internal class for functional components

[gaearon]

For fun, I tried to make a separate internal class for functional components.
Previously this was attempted in #3995.

This is work in progress. I’d like to measure perf implications of this but haven’t had the time yet.
The one feature that I don’t support here is factory components:

function Foo(props) {
  return {
    render() { return <div />
  }
}

They are very inconvenient with our OOP internal hierarchy because we don’t know whether we’re dealing with a functional or a factory component until we call the function at least once.

I assume this is the same reason #3995 got shoot down. However I don’t think I’ve ever seen a “factory component” in the wild and I’m not convinced in the utility of supporting them. Who are their users?

Shallow rendering is also broken in this PR, but I think it’s fixable.

[acdlite]

They are very inconvenient with our OOP internal hierarchy because we don’t know whether we’re dealing with a functional or a factory component until we call the function at least once.

Fiber deals with this by marking these as "indeterminate components" until the first mount, at which point they are re-classified as either functional or class components. Could a similar approach work here, as well?

https://github.com/facebook/react/blob/master/src/renderers/shared/fiber/ReactFiberBeginWork.js#L137

[gaearon]

The problem is “reclassifying” an already created object as another class is weird (even though technically possible), and we don’t want to add another allocation for a delegating instance either.

Fiber doesn’t use classes so doesn’t have this problem.

[acdlite]

Sorry, I wasn't suggesting to literally change the class/prototype of the object, but to use a flag of some kind.

[gaearon]

Having a flag inside CompositeComponent complicates it logic quite a bit (which is how it works now) and causes us to store redundant information (like public instances for functional components).

Moving a flag outside to a "delegating" instance seems like would incur the extra allocation discussed in #3995 (comment). But maybe I misunderstood that comment.

[keyz]

just curious: what are xit and xdescribe?

[vjeux]

A way to quickly disable a test.

There's also fit which is handy as it tells jest (or jasmine) to only run that specific test, so you don't need to comment out everything else when trying to troubleshot a single one.

[vjeux]

Looks like it's not documented on the jest page for some reason: http://facebook.github.io/jest/docs/api.html#content

cc @cpojer

[gaearon]

This is important to document. Also many people will try to use it.only, it will crash, and people will think Jest doesn't support this feature.

@cpojer

[vjeux]

@gaearon it.only is already in master and will be available the next time a release get cut: jestjs/jest#1632

[sophiebits]

My theory was that it's mostly the polymorphic lifecycle lookups that are slow (ex: evaluating inst.componentDidMount is slow) so if we avoid them by having alternate branches for functional components it would be faster. @keyanzhang implemented this in #7369 but we didn't get a chance to do a benchmark (and I didn't have time to review the code).

[keyz]

@spicyj I tried to benchmark it but the script got affected by the splitting of react and react-dom (also the code needs to be rebased). Would you be interested in a PR that updates the benchmark script?

[sophiebits]

@keyanzhang Sure. Maybe it could take both filenames just like measure.py react.js,react-dom.js react2.js,react-dom2.js or something. (Open to better ideas.)

[gaearon]

I just fixed the bench in #7704.