facebookarchive/prepack
1
1
Fork 0
mirror of https://github.com/facebookarchive/prepack.git synced 2024-10-26 23:32:02 +03:00
Code Issues Projects Releases Wiki Activity
2810294260
prepack/test
History
Caleb Meredith 2810294260 Fix React branch serialization issues (#2318) 
Summary:
After adding classes to my fuzzer (which does not use first render mode) I found #2316. This PR fixes #2316 and a related issue I found while working on that. (Each fix is in a separate commit.) The second issue I found is scarier since the compiler passes but we get invalid output.

In the following example I observed an invariant violation:

```js
const React = require("react");

__evaluatePureFunction(() => {
  function Tau(props) {
    return React.createElement(
      "div",
      null,
      React.createElement(Epsilon, {
        a: props.z,
      }),
      React.createElement(Zeta, {
        p: props.h,
      })
    );
  }

  class Epsilon extends React.Component {
    constructor(props) {
      super(props);
      this.state = {};
    }

    render() {
      return React.createElement(Zeta, { p: this.props.a });
    }
  }

  function Zeta(props) {
    return props.p ? null : React.createElement("foobar", null);
  }

  __optimizeReactComponentTree(Tau);

  module.exports = Tau;
});
```

```
=== serialized but not visited values
=== visited but not serialized values
undefined, hash: 792057514635681
  referenced by 1 scopes
      =>_resolveAbstractConditionalValue alternate(#75)=>ReactAdditionalFunctionEffects(#80)

Invariant Violation: serialized 26 of 27
This is likely a bug in Prepack, not your code. Feel free to open an issue on GitHub.
    at invariant (/Users/calebmer/prepack/src/invariant.js:18:15)
    at ResidualHeapSerializer.serialize (/Users/calebmer/prepack/src/serializer/ResidualHeapSerializer.js:2465:17)
    at statistics.referenceCounts.measure (/Users/calebmer/prepack/src/serializer/serializer.js:259:15)
    at PerformanceTracker.measure (/Users/calebmer/prepack/src/statistics.js💯14)
    at ast (/Users/calebmer/prepack/src/serializer/serializer.js:238:38)
    at statistics.total.measure (/Users/calebmer/prepack/src/serializer/serializer.js:169:17)
    at PerformanceTracker.measure (/Users/calebmer/prepack/src/statistics.js💯14)
    at Serializer.init (/Users/calebmer/prepack/src/serializer/serializer.js:136:35)
    at prepackSources (/Users/calebmer/prepack/src/prepack-standalone.js:66:33)
    at compileSource (/Users/calebmer/prepack/scripts/debug-fb-www.js:92:18)
```

Somehow we were visiting `undefined`, but clearly we weren’t serializing it given the source code. Here’s what was happening:

- We’d visit an additional function calling `withCleanEquivalenceSet`.
- The additional function would enqueue an action which visited `<foobar>`.
- Later, we’d execute the `<foobar>` visiting action outside our `withCleanEquivalenceSet` with our default equivalence set.
- The same thing happens with our new root from our `Epsilon` class.
- Except now some effects have been applied that set the `type` for our `<foobar>` React element in our React equivalence set to `undefined`. Since when we created `<foobar>` we modified its `type` property. (Recorded in `modifiedProperties`.)
- Now our `<Zeta>` in `<Tau>` and our `<Zeta>` in `<Epsilon>` share the _exact same_ `<foobar>` thanks to our equivalence set.
- But `<foobar>` has a `type` of `undefined` thanks to the effects we applied. We should be creating a new `<foobar>` since we are in a new optimized function.
- ***Boom!*** We visit `undefined`, but don’t serialize it since the same effects aren’t applied when we serialize.

This test case caught an important flaw in our visiting logic, but it only manifested as an invariant under these very specific conditions. Which is a little scary. In a large example, like our internal bundle, we would of course serialize some `undefined` value but we would have still visited `undefined` instead of the proper type, _and_ we may consider two elements to be equivalent when we shouldn’t since their components may render independently. This issue (I presume) can also affect we bail out on since they create new trees inside the root tree.

While debugging and fixing this issue, I found another with incorrect/suboptimal output that passes Prepack and passes eslint. Given the following input:

```js
require("react");

__evaluatePureFunction(() => {
  const React = require("react");

  function Tau(props) {
    return React.createElement(
      "a",
      null,
      React.createElement("b", null),
      React.createElement(Epsilon, null),
      React.createElement("c", null)
    );
  }

  class Epsilon extends React.Component {
    constructor(props) {
      super(props);
      this.state = {};
    }

    render() {
      return React.createElement("d", null);
    }
  }

  __optimizeReactComponentTree(Tau);

  module.exports = { Tau, Epsilon };
});
```

We get this output:

```js
(function () {
  var _$0 = require("react").Component;

  var _3 = function (props, context) {
    _6 === void 0 && $f_0();
    _9 === void 0 && $f_1();

    var _8 = <_B />;

    var _4 = <a>{_6}{_8}{_9}</a>;

    return _4;
  };

  var _B = class extends _$0 {
    constructor(props) {
      super(props);
      this.state = {};
    }

    render() {
      return _E; // <--------------------------- Incorrect. `_B` may be rendered outside of `_3`.
    }
  };

  var $f_0 = function () {
    _6 = <b />;
    _E = <d />;
  };

  var _6;

  var _E;

  var $f_1 = function () {
    _9 = <c />;
  };

  var _9;

  var _0 = {
    Tau: _3,
    Epsilon: _B
  };
  module.exports = _0;
})();
```

This happened because the React serializer’s `_lazilyHoistedNodes` logic was implemented in a way that doesn’t play well with the interleaving almost random order of the serializer invocation of React lazily hoisted nodes logic.
Pull Request resolved: https://github.com/facebook/prepack/pull/2318

Differential Revision: D8992253

Pulled By: calebmer

fbshipit-source-id: 4a75e5768ffb7887c3a8afa2a0f3f59e7eac266d
2018-07-25 09:55:21 -07:00
..
error-handler Run Prettier for serializer tests too (#2234) 2018-07-11 03:55:11 -07:00
react Fix React branch serialization issues (#2318) 2018-07-25 09:55:21 -07:00
residual Run Prettier for serializer tests too (#2234) 2018-07-11 03:55:11 -07:00
serializer Support string with abstract argument (#2310) 2018-07-23 17:25:02 -07:00
source-maps Run Prettier for serializer tests too (#2234) 2018-07-11 03:55:11 -07:00
std-in fix #1239 - Command-line based syntax errors now print location 2017-12-18 13:56:26 -08:00
test262@e9a5a7f918 Update test262 submodule hash 2018-07-06 14:25:54 -07:00