Summary:
Release notes: adds `reactOptimizeNestedFunctions` flag to enable nested optimized functions from React components
This PR adds output for components that fail to inline (during evaluation process) and adds a flag so we can enable/disable nested optimized functions in React components (from things like unknown array map functions).
Closes https://github.com/facebook/prepack/pull/2054
Differential Revision: D8202653
Pulled By: trueadm
fbshipit-source-id: 9ea95720e5296e602068ca4fe111ab303b8b24fc
Summary:
Release notes: none
This PR is a general spring cleaning of all the React logic. Nothing is fixed, but the logic for many of the aspects of the React reconciliation have been simplified and tidied up.
- I've removed all cases of mutating ReactElements, we now create new ReactElements instead of mutating – because, well, ReactElements are immutable (as are props).
- Removed ReactSerializerState and moved the logic that previous existed into `realm.react`.
- Simplified the branching logic of the reconciler so we no longer need to pass around state and instead use the existing conditional logic.
Manually tested on all our internal bundles are we have no failures.
Closes https://github.com/facebook/prepack/pull/2039
Reviewed By: gaearon
Differential Revision: D8205619
Pulled By: trueadm
fbshipit-source-id: a84c363038086b490d761dbe18711d617058f05c
Summary:
Release note: none
This PR remove `firstRenderOnly` from a bunch of tests that shouldn't have had it and adds a new test for a big with `props` being incorrectly marked for `refuseSerialization` – `props` can be serialialized without the ReactElement, and the test shows how this might happen.
Closes https://github.com/facebook/prepack/pull/2057
Differential Revision: D8203483
Pulled By: gaearon
fbshipit-source-id: fbfb9cc3bbdadaba5884f87319cb5b54e4a0657a
Summary:
Release note: none
Fix lint errors that came our way via an internal check in.
Closes https://github.com/facebook/prepack/pull/2052
Differential Revision: D8197681
Pulled By: hermanventer
fbshipit-source-id: fdc481228239dcc1eeafc904536188e25289162a
Summary: This ensures failing output for Prepack gets proxied into the actual logs in terminal and Sandcastle. That was the case for Jest, but not for Prepack itself. This surfaces the real error in D8188966, for example.
Reviewed By: sophiebits
Differential Revision: D8191668
fbshipit-source-id: d0e3e9ddc1f7b38157fc57a010026de9bb0ec79a
Summary:
Release notes: none
After discussions last night with sebmarkbage, I noticed that we apply defaultProps in the wrong place when the config is partial object or an abstract object. This now uses a helper function, which is injected once for the entire bundle, that handles `defaultProps` cases as a temporal function entry. Added a regression test too.
Closes https://github.com/facebook/prepack/pull/2036
Differential Revision: D8175799
Pulled By: trueadm
fbshipit-source-id: b6bb4f30ee07f0bd9ac07fc91018c26936696f4b
Summary:
Release notes: none
Now that we have snapshotting from Object.assign and ToObject for abstracts, we can now upgrade the ReactElement creation logic, simplifying a bunch of routes which actually resulted in a bunch of components inlining in tests where they didn't before. Furthermore, it allows us to tighten up the Flow typings in the React reconciler and tidy up some of the checks for `key`s or `ref`s logic, which was a bit hard to read before and didn't take into account objects with multiple values in their values domain. This also fixes a bug with abstract conditional component types not getting the deafultProps values correctly.
Closes https://github.com/facebook/prepack/pull/1996
Differential Revision: D8172083
Pulled By: trueadm
fbshipit-source-id: 0a8a1e0631883c92e914aba1308a109b3afda137
Summary:
Release notes: none
An test only was accidentally merged to master.
Closes https://github.com/facebook/prepack/pull/2014
Differential Revision: D8134882
Pulled By: trueadm
fbshipit-source-id: 55194aa25aea409b6163c04f3d222f77c42acc6c
Summary:
Release notes: opt out of optimizing a React component in a tree with `__doNotRender`
This PR makes it possible to opt out of a single component in a React component tree by detecting a `__doNotRender` flag. For example:
```js
function MyComponent(props) {
...
}
MyComponent.__doNotRender = true;
class MyComponent extends React.Component {
render() {
...
}
}
MyComponent.__doNotRender = true;
```
Closes https://github.com/facebook/prepack/pull/2004
Differential Revision: D8123009
Pulled By: trueadm
fbshipit-source-id: d5fc54aea2ea1253e260e41c1fb7d4f2d4717ffd
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
