Summary:
Release notes: none
This fixes an issue that we were experiencing, that turned out to be the correct fix after looking at https://github.com/facebook/prepack/pull/1997. We shouldn't mutate arrays in the reconciliation process, instead we should create new ones when we have new values. Also, we should ensure that we don't mutate ReactElements during reconciliation too, instead we should just create new ones. Mutating existing objects and values breaks down completely when we enter nested effects that need to re-evaluate the same original values, so this fixes a bunch of internal issues we were seeing on our internal FB bundle.
Closes https://github.com/facebook/prepack/pull/1999
Differential Revision: D8105499
Pulled By: trueadm
fbshipit-source-id: 4e6ceb8b02c886c42156a8903c347765f101c840
Summary:
Release notes: all serializer tests now get linted via ESlint for correct output
This PR is pretty gigantic, but almost 99% of it is changing all the serializer tests so they pass the linting rules – which means that there must be no undefined variables or if they are, they are explicitly accesses via `global`. Furthermore, the linting also checks to ensure that all variables are defined before they are used.
Also I added a regression test `conditions3` that further validates some complex cases I was seeing fail in other cases. The ESlint config is also in its own module so it can be shared between `debug-fb-www`.
Closes https://github.com/facebook/prepack/pull/1998
Differential Revision: D8099445
Pulled By: trueadm
fbshipit-source-id: 4a03c57beb51e394bc7b334728090b9fc515eca8
Summary:
Release notes: all globals emitted reference the global identifier
This is a PR to replace https://github.com/facebook/prepack/pull/1894 that fixes#1890. I found a much simpler fix was to ensure we always treat emitted globals as safe for strict mode regardless. I had to change a test that went against this assumption, but we end up with code that better conforms to strict mode without large changes under the hood to Prepack.
Closes https://github.com/facebook/prepack/pull/1911
Differential Revision: D8095772
Pulled By: trueadm
fbshipit-source-id: d4ed65ba8d52cdb0c8f2171d3f3c02bdd265c32d
Summary:
Release note: none
yarn install used to also invoke the build step. This changed a while ago, but this script was not updated accordingly. Apparently no-one noticed until now.
Closes https://github.com/facebook/prepack/pull/1990
Differential Revision: D8081630
Pulled By: hermanventer
fbshipit-source-id: 87a1f6b340010d27a76da39bb5569ffcb3f7a5b8
Summary:
Release notes: adds experimental `react-dom/server` ahead-of-time rendering to string
This PR was part of a small 1-2 day hackathton to see the applicability of creating a server-side renderer based almost entirely to be a 1:1 of `ReactDOMServer` from `react-dom/server` package. This is by no means a full, complete server renderer but is the foundations for us to do further work on this path in the future. Currently, it consists of a single Jest test, to ensure the output of the Hacker News benchmark matches that of the current ReactDOMServer `renderToString` output.
The performance results look very promising (there are some `console.time` lines you can comment out in the test to see the performance for yourself). This implantation essentially compiles away `react` and `react-dom/server` and instead injects runtime helper functions that do things that cannot be statically determined ahead of time.
Lots of features are missing behind `invariant` TODOs. This was done intentionally.
Closes https://github.com/facebook/prepack/pull/1940
Differential Revision: D8075964
Pulled By: trueadm
fbshipit-source-id: 33b3c7ba26b41871ccd15ad8bde4ad257009fed6
Summary:
Release notes: none
When we create an array with `CreateListFromArrayLike`, if the `elementTypes` parameter is left undefined, then skip the check that might throw an invariant if the passed in value is abstract.
Closes https://github.com/facebook/prepack/pull/1986
Differential Revision: D8071410
Pulled By: trueadm
fbshipit-source-id: f5944ecfc928fb9efe9120153e102faec9d5e2b6
Summary:
Release notes: none
This adds the `no-use-before-define` rule to `debug-fb-www` script. `variables` and `functions` are set to false, which allows them to be skipped if defined in a parent scope or if the function is in the same scope (as it gets hoisted), see: https://github.com/eslint/eslint/pull/7948/files. With our current internal bundle and latest master, this brings about a new error that wasn't previously emitted: `'_$Fg' was used before it was defined. (5525:15)`. Fixes https://github.com/facebook/prepack/issues/1982
Closes https://github.com/facebook/prepack/pull/1984
Differential Revision: D8055274
Pulled By: trueadm
fbshipit-source-id: 93a672dd22bce8cd798ebbb7ed90536a847f9ecb
Summary:
This should resolve#415.
Relevant part of the spec here: https://www.ecma-international.org/ecma-262/6.0/#sec-let-and-const-declarations-runtime-semantics-evaluation
For `// 3. Let env be the running execution context’s LexicalEnvironment.`, I assumed that the `env` variable passed to the function already fulfills and it's not necessary to retrieve it here.
I've omitted the `ReturnIfAbrupt(Value)` steps from the spec, as this is also done for the rest of the implementation in this file.
test262 has one additional test pass as a result of this change, though somewhat surprisingly it's for `for`.
I've verified in the command line REPL that destructuring assignment works as expected in the basic case.
Closes https://github.com/facebook/prepack/pull/1967
Differential Revision: D8027013
Pulled By: NTillmann
fbshipit-source-id: 845527bcca55c845aca993fd2c97349efcbd5f59
Summary:
Release notes: the React reconciler now throw a FatalError upon encountering side-effects in a render
This PR revamps the current React system's restrictions for what you can and can't do during the React reconcilation phase. This is a pretty large update but provides much better boundaries between what is "safe" and not "safe", thus reducing the constraints.
1. A new error `ReconcilerRenderBailOut` is thrown if something occurs in the React reconciliation that causes the render to fail and it's a hard fail – no recovering and continuing.
2. If you mutate a binding/object outside the render phase, given the React component render phase is meant to be "pure", a `ReconcilerRenderBailOut` will be thrown.
3. If you `throw` during the React reconciliation phase, again a `ReconcilerRenderBailOut` will be thrown.
In the future, we should maybe loosen the constraints around all this and maybe allow `throw`, but right now it's causing too much friction. We should attempt to make React components render phase as pure as possible – as it results in much better optimizations by a compiler because we can assert far more without things tripping us up.
Another point, regarding (1), is that we should ideally be able to recover from the error thrown in Prepack. The reason we can't and is something that might be a very deep issue in Prepack, is that effects don't properly restore when we have nested PossiblyNormalCompletions at work. Bindings get mutated on records from changes made within `evaluateForEffects` but never get reset when in nested PossiblyNormalCompletion. If I remove all the things that can cause `PossiblyNormalCompletion`s then everything works fine and bindings do get restored. We can remove the constraint on (1) once we've found and fixed that issue.
Closes https://github.com/facebook/prepack/pull/1860
Differential Revision: D7950562
Pulled By: trueadm
fbshipit-source-id: 4657e68b084c7069622e88c9655823b5f1f9386f
Summary:
Release notes: Removing --simpleClosures option.
This feature was never fully implemented and remained buggy.
Instead of having two distinct modes, let's rather try to improve
pointwise what we perceive as inacceptible for the regular way of
why closures work in the serializer.
Closes https://github.com/facebook/prepack/pull/1876
Differential Revision: D7889689
Pulled By: NTillmann
fbshipit-source-id: 782a6754408a9b250d45691274902c6bb2ed6924
Summary:
Release notes: none
This fixes https://github.com/facebook/prepack/issues/1866. The React reconciler previously had somewhat broken logic when dealing with conditional that may require effects being joined together for things such as if statements. This re-uses the logic in `IfStatement` to ensure we get it right. Furthermore, I found a bug in `fb20.js` that failed unless we made sure we throw a `FatalError` in `AbstractObjectValue` where both consequent and alternate values are not found – like done in other methods in `AbstractObjectValue` joining together a value in this scenario actually generated broken output.
Closes https://github.com/facebook/prepack/pull/1874
Differential Revision: D7887489
Pulled By: trueadm
fbshipit-source-id: 64595296e5090b55a1670ab989e6737d7febab55
Summary:
Release note: Fixed code duplication bug
Issue: #1829
When joining up a saved possibly abrupt completion with the current completion (in Functions.incorporateSavedCompletion), the state must be updated with the effects of the normal part of the join. The normal part of the join does not include the joined generator, so that must be explicitly excluded when applying the effects.
The subtle reason for this is that the type of join point is only known to the caller of incorporateSavedCompletion and, depending on the type of join point, some parts of the joined completion may be excised from the join and put back into a saved completion. This means that only the caller knows exactly which parts of the abrupt completions should have their effects (and generators) reflected in the current state.
Closes https://github.com/facebook/prepack/pull/1834
Differential Revision: D7848979
Pulled By: hermanventer
fbshipit-source-id: 80e61692ea8f3b57930b5125178b560fa3d47af5
Summary:
Release notes: adds ReactDOM mocks to fb-www compatibility mode
This PR aims to do a bunch of things:
- Tidy up the spaghetti React mock logic by adding in some helper functions
- Add ReactDOM mocks, pathing the way for greater optimizations for first render
- Swap the `Get` to a `getProperty` in `isReactElement` function
- Make the fb-www mocks not ignored by eslint and fix the eslint issues after doing so
Closes https://github.com/facebook/prepack/pull/1835
Differential Revision: D7845112
Pulled By: trueadm
fbshipit-source-id: 7173c543da6af2c20a38fbc70f962825647f8f3e
Summary:
Release notes: none
This PR fixes https://github.com/facebook/prepack/issues/1819. It expands on the existing invariant check in `AbstractObjectValue` by also allowing abstract object values to pass through.
Closes https://github.com/facebook/prepack/pull/1824
Differential Revision: D7829938
Pulled By: trueadm
fbshipit-source-id: 7e198bc626b4fd2737d0ef9d2a7207bd17b0b02a
Summary:
Release note: none
Enable test fb-www 12.
While I'm at it, also update package.json to get rid of deprecated -- option and thus shut up a warning about it in CircleCI.
Closes https://github.com/facebook/prepack/pull/1817
Differential Revision: D7824465
Pulled By: hermanventer
fbshipit-source-id: ed88bd1ed8b02c560faf58be24a6592b3e55b86b
Summary:
Release note: none
Enable a test that is now working because of PR #1813.
Closes https://github.com/facebook/prepack/pull/1814
Differential Revision: D7823202
Pulled By: hermanventer
fbshipit-source-id: fb4bf726ffac55094851baf6eb19527671e12ad9
Summary:
Release notes: none
This should fix https://github.com/facebook/prepack/issues/1803 and extends from the unsuccessful attempts made in https://github.com/facebook/prepack/pull/1809 to fix this.
When we apply the `priorEffects` inside of the `evaluateForEffects` on TryStatement, we have to ensure that the `canBeApplied` flag is reset (as the bindings on the Realm get restored by `evaluateForEffects`).
Closes https://github.com/facebook/prepack/pull/1811
Differential Revision: D7820907
Pulled By: trueadm
fbshipit-source-id: 248cc0f21be0044de8641e79c5985c7259a88629
Summary:
Release notes: none
This adds more validation that something definitely is a ReactElement, as found from the issue https://github.com/facebook/prepack/issues/1802. Unfortunately, the test in that case does not pass, it fails due to an invariant:
`Invariant Violation: an abstract value with an identifier "_$1" was referenced before being declared`
Closes https://github.com/facebook/prepack/pull/1810
Differential Revision: D7813932
Pulled By: trueadm
fbshipit-source-id: e431887002da83389da4062cfe00afbdc42d21c8
Summary:
Release notes: none
When digging deeper into detecting write conflicts with React component trees, I found that PossiblyNormalCompletions were not handled properly, so this PR ensures that the effects tree is recursively traversed so all modified bindings are collected, even from PossiblyNormalCompletions.
Update:
With all the latest changes, all tests in this PR now pass, so it's ready to be reviewed again.
Closes https://github.com/facebook/prepack/pull/1742
Differential Revision: D7813778
Pulled By: trueadm
fbshipit-source-id: 425ec505709c83081c7eef3bec9bf95bd651299e
Summary:
Release notes: none
This PR fixes an issue where the React reconciler incorrectly assumed that `createFromConditionalOp` always returned an `AbstractValue`. This changes that and adds the failing test for a regression test.
Fixes https://github.com/facebook/prepack/issues/1804.
Closes https://github.com/facebook/prepack/pull/1807
Differential Revision: D7813026
Pulled By: trueadm
fbshipit-source-id: 5af963fb0e2351d073df554cda33b979d52c9e60
Summary:
Release notes: None
This fixes#1798, and it almost fixes#1797 by reducing it to #1799, and it unblocks #1794.
When the visitor looks at (initial) function bindings, it does so with additional functions still applied if that additional function happens to be the first thing that pull on the function value.
This is being fixed, as well as generally turning _withScope uses into _enqueueWithUnrelatedScope calls which ensure that the right effects are in place.
Added additional output to the serializer runner - it now shows a summary of which tests failed, ordered by their size.
Added regression test.
Closes https://github.com/facebook/prepack/pull/1806
Differential Revision: D7805532
Pulled By: NTillmann
fbshipit-source-id: 274859e9f6f74ebfc4decc37d984cedcbedf990c
Summary:
Release notes: none
This PR provides a mechanism for the React reconciler to understand newly created arrays from maps with unknown properties, in this this first case this is only for `Array.from`, `Object.keys`, `Array.prototype.map`, `Array.prototype.filter`, `Array.prototype.reduce` (with constraints), `Array.prototype.slice` and `Array.prototype.concat`.
When we have something completely abstract, like `props.items`, and it's been wrapped in `Array.from`, we can assume that (in a pure render) that it's safe to emit the `Array.from` and make the array with an unknown property that is an abstract widened hint. The reconciler can see this and perform a nested optimized closure to the map callback.
In cases where the third argument `thisVal` is passed to array methods, we bail-out.
Closes https://github.com/facebook/prepack/pull/1770
Differential Revision: D7795295
Pulled By: trueadm
fbshipit-source-id: 3396330da6d4ebe2ffdd1f1610173eb482df9081
Summary:
Release notes: Collect fine-grained statistics on memory usage
This refactoring does a few things:
- Introduce a much nicer `measure` abstract to wrap computations for which we want to measure something
- Measure not only time, but also heap usage
- Project data to legacy statistics file format.
- Limit scope of intermediate serialized information to further reduce memory usage
Closes https://github.com/facebook/prepack/pull/1782
Differential Revision: D7765817
Pulled By: NTillmann
fbshipit-source-id: e6ceb05146490168c379fd8d7d3f1ecffd73db01
Summary:
Release notes: None
The option works just like for the Prepack CLI.
Closes https://github.com/facebook/prepack/pull/1785
Differential Revision: D7787080
Pulled By: NTillmann
fbshipit-source-id: 04e9cd5b7323e5cf55e1d0ff34a4a3b774df0148
Summary:
Release notes: none
This PR came about from trying to fix https://github.com/facebook/prepack/issues/1771. This PR fails on the tests still, so I need some help getting them resolved.
- `_emitProperty` should account for the fact a `key` can be an `AbstractValue`, thus needs to serialize the value.
- `visitObjectPropertiesWithComputedNames` in the visitor and `_getNestedAbstractValues` in the serializer both need to take into account cases where `alternate` and `consequent` might not be `AbstractValue`s.
Closes https://github.com/facebook/prepack/pull/1781
Differential Revision: D7777483
Pulled By: trueadm
fbshipit-source-id: 1b94489ec9c800a68414031e0bc63b06061fe559
Summary:
Release notes: none
When doing `Object.assign` in `getDerivedStateFromProps`, we need to add back in the fallback for when it might fail otherwise we end up triggering invariants. This fixes https://github.com/facebook/prepack/issues/1773
Closes https://github.com/facebook/prepack/pull/1780
Differential Revision: D7773190
Pulled By: trueadm
fbshipit-source-id: 2539f01cfde3a65136a6e9947a4620dfc81a371a
Summary:
Release notes: none
There was a case where an invariant was firing in the hoisting logic due to a binding value being undefined. This PR stops the invariant from firing in such cases, and includes the regression test. I also found another issue in the evaluatedReactNode debugging output and fixed that too so it was easier to see what was going on with this and other issues.
This fixes https://github.com/facebook/prepack/issues/1775
Closes https://github.com/facebook/prepack/pull/1779
Differential Revision: D7763057
Pulled By: trueadm
fbshipit-source-id: 61c866f7cb7322f5d884262adb85613c97cfa4ca
Summary:
Release notes: none
This fixes bugs around detecting independent dangling `this` keywords in methods of a React class component and correctly bailing out upon finding them. This is typically used in cases where we'd want to bail out, for example:
```js
let self = this;
let handleClick = function () { console.log(self.x) };
return <div onClick={handleClick} />
```
or:
```js
let handleClick = function () { console.log(this.x) };
return <div onClick={handleClick.bind(this)} />
```
Closes https://github.com/facebook/prepack/pull/1777
Differential Revision: D7757435
Pulled By: trueadm
fbshipit-source-id: b06fe29a3ac4a44bdf2b8a7c6e9457e5801b32b7
Summary:
Release notes: fixes a regression where on* events would not be stripped in React firstRender mode
This PR fixes a recent regression in the stripping of onEvent names in React compiler's firstRender mode. The associated tests have been updated so the snapshots should pick up this regression in the future.
Closes https://github.com/facebook/prepack/pull/1774
Differential Revision: D7747737
Pulled By: trueadm
fbshipit-source-id: e0dd65a7e4d73844c499d1d8b2f9ae8046bfec2b
Summary:
This adds a React-specific internal test for Prepack to fbsource. It's meant to be land-blocking.
I checked in a manually edited bundle that has no dependencies except `react` and `react-dom` (so it's not too difficult to maintain). I also checked in a snapshot test for what it should render.
The test step consists of running the Jest test (to verify it works *before* Prepack), then using the existing `debug-fb-www` command to produce a bundle, and finally running the same Jest test again.
Currently this only checks the initialization path. When render path serializer bugs are fixed, we can run the same bundle in the render mode as well (and catch further regressions).
Reviewed By: NTillmann
Differential Revision: D7675812
fbshipit-source-id: 99fe3bd700ae3557ee6245f1e8e1e835399625b5
Summary:
Release notes: none
This adds the functionality to fold/inline ReactRelay components via `createFragmentContainer`. This is for first-render only for now.
Closes https://github.com/facebook/prepack/pull/1745
Differential Revision: D7686225
Pulled By: trueadm
fbshipit-source-id: 7eb546ff70e92b60bcc1abb69c0cc5091926c012
Summary:
Release notes: none
When a `null` or `undefined` value is passed as the `config` for `createElement`, then we need to create an empty object to be used instead.
Closes https://github.com/facebook/prepack/pull/1759
Differential Revision: D7686087
Pulled By: trueadm
fbshipit-source-id: a8eb917f38ca3afe881af08d07fe8be0943b0a1f
Summary:
Release notes: none
Ensure we apply the nested closure effects in the right order. This was the underlying bug after doing much digging.
Furthermore, I added case where this fails. I also found that the ReactElement code was causing issues at one point (I forgot to add the props, type, ref and key as dependencies), but that wasn't needed in the end for this fix, but I thought it best to keep those fixes in.
I also had to make a change to the JSON tester as it was incorrectly failing a test because the function closures were not 1:1, which they're not meant to be.
Closes https://github.com/facebook/prepack/pull/1743
Differential Revision: D7686048
Pulled By: trueadm
fbshipit-source-id: 8cf7faed51c86720e519abbbad209820e7e02dc4
Summary:
Release notes: none
When working on internal projects, I found cases where the `getDerivedStateFromProps` would throw a FatalError because the partial state wasn't an abstract object value or object value. In this case we can see that the abstract value is a conditional and handle the paths to ensure we get the correct results without failing.
Closes https://github.com/facebook/prepack/pull/1737
Differential Revision: D7615094
Pulled By: trueadm
fbshipit-source-id: 5c868a46936a108c277ac52c2675feabc1f4df8c
Summary:
Release notes: Introducting new option --invariantLevel NUMBER and dropping --omitInvariants
The new default is invariant level 0, which corresponds to the old --omitInvariants.
Level 1 is roughly what we used to do: Check invariants on (derived) abstract values.
Level 2 implements #1180: Checking that all accesses to built-ins do what Prepack expects them to do, including checks for when properties on built-ins are absent.
Level 3 adds checks for internal consistency, basically an internal debug level.
The serializer tests now run with the highest invariant level by default. The added invariants found a few issues that got addressed, including:
- Prepack exposes a TypedArray prototype, which is not a real thing. It's now marked as `__refuseSerialization`, and no invariants are emitted for such things.
- Global variables / properties on the global object are special. Those are not yet handled at level 2.
- Accesses to Prepack magic functions that generally start with `__` should not produce invariant checks.
- Magic code generation for loops should not take into account objects that are `__refuseSerialization`.
- All invariant statements get a unique id to make it easier to locate them.
- I marked some test cases which depend on counting occurrences in the output as "// omit invariants", as the additional invariants increased some such counts.
As part of testing, I also found it necessary to make the --invariantMode more useful; it now also allows specifying nativeLoggingHook which is the preferred way of logging in React Native.
To reduce the number of checks by a few orders of magnitude in practice, each property is only checked on first access. This is tracked by a global variable `__checkedBindings`.
This pull requests incorporates all aspects of the #1709 (which I abandoned).
Closes https://github.com/facebook/prepack/pull/1724
Reviewed By: simonhj
Differential Revision: D7575091
Pulled By: NTillmann
fbshipit-source-id: 585cd224ce66882f8e5f27d88c1ad08afeb96ee1
Summary:
Release notes: none
This PR adds some sanity around React components roots that when render, mutate the same of bindings as other React component roots. This is currently not supported and thus safely bails out and continues.
Closes https://github.com/facebook/prepack/pull/1736
Differential Revision: D7600373
Pulled By: trueadm
fbshipit-source-id: fe2b2b3055fa3c1b28668da6d67bca3826431eb6
Summary:
Release notes: Adds`createRef` and `forwardRef` to the React compiler
This PR adds both features from React 16.3, including full inlining of forwardRef functions.
Closes https://github.com/facebook/prepack/pull/1731
Differential Revision: D7599780
Pulled By: trueadm
fbshipit-source-id: 50458546c7a4374baea7fa80feb21262dd5692af
Summary:
Release notes: adds React 16.3 `getDerivedStateFromProps` support to the React compiler
This PR adds support for the React 16.3 `getDerivedStateFromProps` lifecycle event on class components. I've also updated the React testing dependencies to 16.3.1. I also extended the `componentWillMount` logic to support `UNSAFE_componentWillMount` too.
Closes https://github.com/facebook/prepack/pull/1730
Differential Revision: D7586378
Pulled By: trueadm
fbshipit-source-id: 895e74ca86eb9ff9ad04aa93f021c107f08a7b9f
Summary:
Release notes: none
Some internal changes to ensure `Bootloader.loadModules()` global stays as is and doesn't get destructured. Also changed the incorrect usage of `context.$Realm.generator.derive` in the mocks.
Closes https://github.com/facebook/prepack/pull/1726
Differential Revision: D7578301
Pulled By: trueadm
fbshipit-source-id: dd7dce3e46ff01b10eb9206a1ed5d94871ae93fd
Summary:
It seems like it's unsafe to rename on www (it breaks Haste) so we should add it to the whitelist of globals.
It's both a function *and* it has properties that are functions, but for now I'll model it as an object since we never use it directly. In fact it seems like www transforms direct calls on it to `fbt._` calls or something like that so maybe keeping it as an object is right. We don't have a single direct call to `fbt()` as a function in our bundle.
This fix unblocks testing the bundle initialization in Jest www environment.
Closes https://github.com/facebook/prepack/pull/1725
Reviewed By: trueadm
Differential Revision: D7568551
Pulled By: gaearon
fbshipit-source-id: bdfc7c949d6be698e1f58a85c1e39f8b6d5b6bae
Summary:
Release notes: None
Apparently, Flow does not necessarily report the largest cycle first, which our script assumed.
So now the script looks at all cycles and reports the largest.
Closes https://github.com/facebook/prepack/pull/1693
Differential Revision: D7480758
Pulled By: NTillmann
fbshipit-source-id: 0f55f22ea1442ba1c842ad44db6bf91bd774306d
