Summary:
Release note: Fix test runner and don't swallow invariant failures
The recent changes to make the serializer test runner deal with promises, caused it to list every test that follows a failing test as also failing.
I've also noticed that invariant failures can get swallowed. And as a result of that some test cases that should have failed with false invariants showed up as passing.
Those failures were recently introduced by PR #2134. It boils down to a single line fix, which is also included with this.
Closes https://github.com/facebook/prepack/pull/2155
Differential Revision: D8629298
Pulled By: hermanventer
fbshipit-source-id: 467f7efdf119ea9d29083d4e41054e583b38a1ff
Summary:
Release notes: none
To make https://github.com/facebook/prepack/pull/2096 easier to review, this pulls out the tidy ups from that PR.
- fix typo on function name `mergeAdjacentJSONTextNodes`
- move ReactDOM mock logic into its own file
Closes https://github.com/facebook/prepack/pull/2163
Differential Revision: D8625051
Pulled By: trueadm
fbshipit-source-id: 5345cc26f5c613b4afbb1c768213fd84caf5ebab
Summary:
Release notes: None
We used to find random bugs where the `computed` flag wasn't set right.
But even if it's set correctly, we sometimes generated suboptimal code,
e.g. `a["0"]` for an array access.
I reviewed all occurrences of `t.memberExpression`, and redirected all that involve
potentially interesting computations to a new helper function `memberExpressionHelper`
which does "the right thing". This function should always be used.
Added regression test for an array access that used to be suboptimal (observed in InstantRender).
Closes https://github.com/facebook/prepack/pull/2162
Differential Revision: D8625006
Pulled By: NTillmann
fbshipit-source-id: f9211260c08d9195a9f4cc48502cba0794fe84f9
Summary:
Release notes: adds basic ES2015 arrow function serialization
This allows Prepack to serialize back to arrow functions and also traverse them correctly when understanding `this`.
Closes https://github.com/facebook/prepack/pull/1650
Differential Revision: D8623368
Pulled By: trueadm
fbshipit-source-id: 9be7849770148b67e5f17eee88a633aac906b6e2
Summary:
Release note: none
This reverses PR #2136, which Dan has noted to lead to a 3x slowdown for an internal bundle. In order to fix the problem that Nikolai's PR addressed, the original code has been modified to not purge any local bindings that could possibly have been captured in a closure.
I expect that most effects will not be in a context that contains a closure, so this should help. It would be nice, however, to get some feedback on whether this is actually the case.
Closes https://github.com/facebook/prepack/pull/2152
Differential Revision: D8617877
Pulled By: hermanventer
fbshipit-source-id: 8e907ba1dded0428388fd42132658555e3b8e82e
Summary:
Release notes: standard for loops and while loops have a recovery mechanism in pure scope
This PR provides a bail-out recovery mechanism in pure scope for FatalErrors thrown and caught from for/while loops. Until now, if a FatalError occurs from trying to evaluate abstract for/while loops, we'd have to recover at a higher point in the callstack, which wasn't always possible (the loop may be at the root of a function/component).
The ideal long-term strategy is to properly model out the different cases for loops, but this is a complex and time-consuming process. This PR adds a recovery mechanism that serializes out the original for loop, but within a newly created wrapper function containing the loop logic and a function call to that newly created function wrapper. This allows us to still run the same code at runtime, where we were unable to evaluate and optimize it at build time.
For now, this PR only adds recovery support for standard `for` and `while` loops (as they go through the same code path). We already have some basic evaluation for `do while` loops, but trying to adapt that code to work with the failing test case (https://github.com/facebook/prepack/issues/2055) didn't work for me – we have so many strange problems to deal with first before we can properly handle that issue.
In cases where the loop uses `this`, the context is found and correctly called with the wrapper function. In cases of usage of `return`, `arguments` and labelled `break/continue` we bail out again as this is not currently supported in the scope of this PR (can be added in a follow up PR, but I wanted to keep the scope of this PR limited).
For example, take this failing case on master:
```js
function fn(props, splitPoint) {
var text = props.text || "";
text = text.replace(/\s*$/, "");
if (splitPoint !== null) {
while (text[splitPoint - 1] === "\n") {
splitPoint--;
}
}
return splitPoint;
}
```
This serializes out to:
```js
var _0 = function (props, splitPoint) {
var __scope_0 = new Array(1);
var __get_scope_binding_0 = function (__selector) {
var __captured;
switch (__selector) {
case 0:
__captured = [_G, _E];
break;
}
__scope_0[__selector] = __captured;
return __captured;
};
var _C = function () {
var __captured__scope_1 = __scope_0[0] || __get_scope_binding_0(0);
for (; __captured__scope_1[1][__captured__scope_1[0] - 1] === "\n";) {
__captured__scope_1[0]--;
}
};
var _$0 = props.text;
var _$2 = (_$0 || "").replace(/\s*$/, "");
var _6 = splitPoint !== null;
var _G = _6 ? void 0 : splitPoint;
var _E = _6 ? void 0 : _$2;
if (_6) {
(__scope_0[0] || __get_scope_binding_0(0))[0] = splitPoint;
(__scope_0[0] || __get_scope_binding_0(0))[1] = _$2;
var _$5 = _C();
}
var _$6 = (__scope_0[0] || __get_scope_binding_0(0))[0];
return _$6;
};
```
Furthermore, an idea that might be a good follow up PR would be to break the wrapper function into two functions, depending on some heuristics. If we can detect that the loop body does not have any unsupported side-effects like writing to variables that are havoced etc, we can then tell Prepack to optimize the inner function wrapper for the loop body. That way, at least some parts of the loop get optimized (the outer bindings will be havoced before this point, so it should work okay). I think I suggested this in person with hermanventer and sebmarkbage in Seattle as a potential way to optimize complex loops that we can't compute the fixed point for right now.
Fixes#2055
Closes https://github.com/facebook/prepack/pull/2118
Differential Revision: D8410341
Pulled By: trueadm
fbshipit-source-id: ee7e6b1bc1feadf0c924e4f82506ca32ca1dadc9
Summary:
Release notes: none
Fixes a bug where the argument list gets mutated and padded with undefined values, so instead we slice it a copy of it so we don't mutate the same array and pass the mutated array instead. Fixes https://github.com/facebook/prepack/issues/2153.
Closes https://github.com/facebook/prepack/pull/2156
Differential Revision: D8610746
Pulled By: trueadm
fbshipit-source-id: 1b6f37693323c813d60f43ab7e9977a99f86ff47
Summary:
Release notes: none
As we're updating state on a final object, in this case `state`, ensure we use `hardModifyReactObjectPropertyBinding`. Unblocks https://github.com/facebook/prepack/pull/2137.
Closes https://github.com/facebook/prepack/pull/2149
Reviewed By: hermanventer
Differential Revision: D8584676
Pulled By: trueadm
fbshipit-source-id: f23c77a5091b7c1023a48a5d29353bf9cdc06d65
Summary:
Release notes: in pure mode, allow Object.assign to continue evaluation rather than throwing FatalErrors
This PR adds bail-out support of `Object.assign` sources within the `Object.assign` implementation. This allows us to continue evaluation in pure scope without bailing out of the Object.assign entirely (like we currently do) upon a FatalError occurring. Given we have snapshotting in `Object.assign`, we don't need to havoc the sources.
This allows us to inline and evaluate far more within our internal bundles. Fixes https://github.com/facebook/prepack/issues/2064
Closes https://github.com/facebook/prepack/pull/2068
Differential Revision: D8579441
Pulled By: trueadm
fbshipit-source-id: 0e97b54e0c8af63f1e3cd08fca6c7f375ee3c615
Summary:
I haven't written the next parts yet but I figured let's put a link on the website so this doesn't get lost.
I think even the details there are useful enough for users and new contributors.
<img width="1513" alt="screen shot 2018-06-21 at 3 57 52 pm" src="https://user-images.githubusercontent.com/810438/41727325-f9ba32da-756b-11e8-94f0-c83b71df859b.png">
I put it at the top of Getting Started because IMO you'll want to read this (or at least see it) before trying to run Prepack.
I initially planned to put it in the docs themselves but only today realized there's no build process. I don't want to convert the guide to HTML so I'd rather keep it as a gist (and update it if necessary) in the meantime.
Closes https://github.com/facebook/prepack/pull/2147
Differential Revision: D8577403
Pulled By: gaearon
fbshipit-source-id: 2f7f64a0a4413066739b12c50b4d21779a16a568
Summary:
Release notes: none
This PR contains some fixes and small tidy ups that would have been in #2138. There was a bug to do with the React props equivalence system and snapshots that was found when https://github.com/facebook/prepack/pull/2137 was merged (along with the fix I commented to on that PR). This PR unblocks https://github.com/facebook/prepack/pull/2137 to land as it should now pass our internal bundle test.
Closes https://github.com/facebook/prepack/pull/2146
Differential Revision: D8556958
Pulled By: trueadm
fbshipit-source-id: dd774de76059e00af1dff305dd3618c3bfed06ea
Summary:
Release Notes: Add support for multiple input files for the debugger, in both debugger-cli and prepack-cli (used by debugger when launched from Nuclide).
Closes https://github.com/facebook/prepack/pull/2130
Differential Revision: D8554746
Pulled By: caiismyname
fbshipit-source-id: ffba51d0bc6f2b7fa171072a358bdfc5c911a2d7
Summary:
Release notes: Fixes to leaked declarative bindings
Leaked declarative bindings need to be accessed out-of-band as far
as the serializer is concerned which otherwise only puts final
values into locations.
To make that work, whenever two states are merged where a
declarative binding in one has been leaked but not in the other,
this now gets harmonized by also recording the materialization
side effect into the generator of the other.
The result is that a binding always ends up being fully leaked or not.
For leaked mutable bindings, the tracked value becomes irrelevant, and
is now truly made unavailable. For leaked immutable bindings, the last
value is retained, and we no longer need the strange `leakedImmutableValue` field.
Leaked bindings are now referentialized as simple variables, bypassing the
delayed captured-scope logic. This produces nicer more efficient code, but,
at least for now, has the downside that it rules out sharing of function bodies.
This change is a step towards the (to be publicly documented)
long-term plan to clean up Prepack's handling of leaking and havocing.
(Similar to what this change does for declarative bindings, we'll also must
revisit what happens for leaked object properties. But that's for later.)
This fixes#2122 (issue havocing value in conditional) and adds regression test.
This fixes#2007 (Conceptual issue with havoced bindings) and adds regression test.
Closes https://github.com/facebook/prepack/pull/2125
Differential Revision: D8549299
Pulled By: NTillmann
fbshipit-source-id: 28c3681beafd3890668b72a828f59582c636a6c9
Summary:
Release notes: none
After studying the issues in https://github.com/facebook/prepack/pull/2137, I understood that we currently make some assumptions in terms of the ReactElement equivalence system that are incompatible with what we want from Prepack going forward. This PR aims at fixing those assumptions so they fall more into line with the changes https://github.com/facebook/prepack/pull/2137.
This PR makes the following changes:
- All React equivalence is now handled explicitly in the ReactSet/ReactElementSet/ReactPropsSet to ensure all the logic is located together as one and not sprinkled around the codebase.
- We use `hardModifyReactObjectPropertyBinding` to modify final objects that React controls rather than `Properties.Set`. This function bypasses the normal route of putting the property changes on the effects – thus making it so the property changes are permanent and cannot be reverted. Given the object is immutable, there should never be a case where the property may change anyway – we only change the property as part of the equivalence phase and during reconciliation where the React internals are at play. This is only safe for objects explicitly created by React and marked as final by React – no other object can go through this codepath. Any attempt to use this dangerous function on a non-final object will result in an invariant.
- Various bug fixes were fixed that were found when #2137 was merged and existing tests failed.
- Removed all the logic around `makeNotFinal` and use `hardModifyReactObjectPropertyBinding` instead.
Closes https://github.com/facebook/prepack/pull/2138
Reviewed By: gaearon
Differential Revision: D8547558
Pulled By: trueadm
fbshipit-source-id: 192da9169947adadc41f43997dfbe0adb73cec3b
Summary:
Release note: none
Attempts adding support for promises in test-runner.js. `inspect` can now return Promises, which will be resolved before checking it value.
Closes https://github.com/facebook/prepack/pull/2115
Differential Revision: D8550825
Pulled By: hermanventer
fbshipit-source-id: 3f9781f6e34016dcb61bd436400a1bfb51932452
Summary:
Release notes: None
For historical and apparently no longer needed reasons,
we used to forget about modified bindings when execution
leaves scopes.
However, this means that the binding no longer properly participates
in state tracking, which is conceptually dubious.
While by itself, it didn't seem to cause much (or any?) harm,
it turned out to be a blocker for #2125, where the initial state
of a binding becomes important when joining partially leaked bindings.
Closes https://github.com/facebook/prepack/pull/2136
Reviewed By: hermanventer
Differential Revision: D8528939
Pulled By: NTillmann
fbshipit-source-id: 0890c32d55f6b498cb4613171eee54ff9b445730
Summary:
Release Notes: None
Follow up on #2110, this PR makes progress towards unifying PossiblyNormalCompletion and ForkedAbruptCompletion as well as unifying completion and effects.
Introduces `updateConsequentKeepingCurrentEffects` for a common pattern of updating one of the completions for a PNC/FAC and fixing up the effects.
Closes https://github.com/facebook/prepack/pull/2134
Differential Revision: D8525738
Pulled By: cblappert
fbshipit-source-id: c415eea47cd786e39820138f84213c2cc9d5e891
Summary:
Release Notes: None
In an effort to unify Completions and Effects (many completions are tied to effects whose result should be the completion), I change evaluateForEffects to return a `NormalCompletion` whereever it would otherwise return a `Value`. Now all `EvaluationResults` are `Completions` or `References`.
Fairly mechanical refactor (except slightly tricky because PossiblyNormalCompletions are NormalCompletions, so instead of checking `instanceof Value` you should check `instanceof NormalCompletion && ! instanceof PossiblyNormalCompletion`).
Closes https://github.com/facebook/prepack/pull/2110
Differential Revision: D8431970
Pulled By: cblappert
fbshipit-source-id: d70d6c5d4100eec52f60b8e4328f2a0b4afabbed
Summary:
Release notes: none
There was a bug, where React abstract object values with the same intrinsic name were incorrectly being de-duped and re-used. This is a normal expected Prepack optimization, but it actually breaks React components that expect to be able to create many values with the same intrinsic name. We now mark such objects and ensure they do not correctly return `true` from `equals()`. Fixes https://github.com/facebook/prepack/issues/2089
Closes https://github.com/facebook/prepack/pull/2126
Differential Revision: D8448171
Pulled By: trueadm
fbshipit-source-id: d21fddd70576e6e55c41070ac43ce2f58a059e7a
Summary:
Release notes: none
This PR moves the logic of sanitizing ReactElements for `firstRenderOnly` mode to the serialization phase instead of during reconciliation. The plan is to create a follow up PR that introduces a new phase after the visitor phase, that skips values that were detected as dead code once the visitor stage finishes, but that's not in scope for this PR.
Closes https://github.com/facebook/prepack/pull/2123
Differential Revision: D8435440
Pulled By: trueadm
fbshipit-source-id: f442525e663a7bc0f9608b96eb7d7a2389a707b7
Summary:
Release notes: fixes serialization of bindings when shared between optimized functions
This PR fixes a long standing bug with nested optimized functions where bindings would incorrectly serialize to the main body scope when the binding was shared between nested optimized functions and their parent optimized function scopes. This allows us to not skip 2 React tests and I also added a serializer regression test that demonstrates the same problem so we don't regress.
Closes https://github.com/facebook/prepack/pull/2117
Differential Revision: D8408712
Pulled By: trueadm
fbshipit-source-id: 0e0a306f9195fcb5b57fa7f8cef1c65dbef4998f
Summary:
Release notes: None
In particular, no longer claim that there were "errors" when there in fact none.
Closes https://github.com/facebook/prepack/pull/2104
Differential Revision: D8371246
Pulled By: NTillmann
fbshipit-source-id: b33519bf8a2b53150b24ea6f7db9b43b20e7dd1f
Summary:
Release Note: none
This is regarding #2072
Instead of spitting back Invariant Violation: undefined in this case, it will give back the trace error.
Also the Invariant Violation: undefined is misleading. The undefined is coming from the invariant format parameter when format isn't supplied. The fix is to add an empty string to format so it won't show undefined.
Questions: would you want to separate that throw into a separate util method like invariant?
Closes https://github.com/facebook/prepack/pull/2106
Differential Revision: D8394381
Pulled By: NTillmann
fbshipit-source-id: 3ed418c335b7db6d88bb1cca5dcc9ac69fed6b5d
Summary:
Release notes: none
When we create temporals that are pure, let's mark them as pure and also skip creating invariants for them too. Making these changes allows the serializer to not emit temporals that never end up getting used, reducing the output and also making it easier to debug the output (less spam of temporals).
Closes https://github.com/facebook/prepack/pull/2097
Differential Revision: D8329043
Pulled By: trueadm
fbshipit-source-id: f1ef4185793115dd7982b8162a7e3dc4038a15f9
Summary:
Emit PrePack info into the generated bundle. The intention is for this to include information about enabled prepack options. Currently it only includes a flag if lazy objects are enabled.
Its emitted as global which is somewhat intrusive so its guarded by a flag currently and defaults to false.
Example:
```
var __PREPACK__ = {lazyObjects: false}
```
Reviewed By: NTillmann
Differential Revision: D8321765
fbshipit-source-id: 03a41b2cef84ea44c02c71e233b407e3ddc1197d
Summary:
Release notes: none
This is a long standing fix, where ReactElements should be created lazily but are currently not done so. To do this, we now use the `temporalAlias` code paths on lazy branched ReactElements to ensure they are emitted as temporal entries when we "branch" (i.e. conditional) and have many possible routes where ReactElements may or may not be created.
Fixes https://github.com/facebook/prepack/issues/2113. Depends on https://github.com/facebook/prepack/pull/2112
Closes https://github.com/facebook/prepack/pull/2107
Differential Revision: D8383327
Pulled By: trueadm
fbshipit-source-id: bbcff45ddd07406b18bcddce2de0279fb52da1a1
Summary:
Release notes: improves ReactElement creation by re-using ReactElement nodes where possible
We already have a pretty good ReactElement equivalence system to re-use ReactElements. We weren't using it to its full power though. When we have nested ReactElements, we don't use the equivalence system at all, meaning we duplicate lots of ReactElements when we can re-use.
This is a nice performance optimization. I've added a test that should prevent regressions here too.
Closes https://github.com/facebook/prepack/pull/2114
Differential Revision: D8381595
Pulled By: trueadm
fbshipit-source-id: befabd04326a162f8ab6a00e089cdada052cf6c8
Summary:
Release notes: none
When working on https://github.com/facebook/prepack/pull/2107, I noticed that we weren't cloning `temporalAlias` values, which is the wrong approach. We instead were re-using them on cloned ReactElements. The current approach worked fine with concrete ReactElement values, but when we introduce temporal ReactElement values that have dependencies on a shared `temporalAlias`, we run into very complex problems.
This PR makes it so we can clone `temporalAlias` values on `ObjectValues` (used for snapshotting). This only used for React code – so it should not affect the other parts of Prepack. I've tested this on our internal bundle and our tests and they all pass, we create more code – but this is a *better* approach. We can remove the bloated code at a later point, but this aims to keep our approach consistent.
Closes https://github.com/facebook/prepack/pull/2112
Differential Revision: D8379812
Pulled By: trueadm
fbshipit-source-id: 04bf133be7a97cec3389a99584be1c042371ee07
Summary:
Release notes: Speeding up visiting phase.
While there was already some batching in place, lumping together all actions associated with a particular final generator, the batching did not take into account the inherent tree structure of the generator effects. This is now being fixed.
Also added logging when in React verbose mode.
This change dramatically speeds up internal benchmarks.
However, the fixed point computation is still an intolerable bottleness.
Future changes can or should include some dependency tracking during the fixed point computation. Right now, each item is a block box, and it is not clear whether it needs to be re-run, so it is re-run.
(Created issue #2111 to track this opportunity.)
This speeds up an internal React benchmark from 261s to 193s.
Closes https://github.com/facebook/prepack/pull/2105
Differential Revision: D8370641
Pulled By: NTillmann
fbshipit-source-id: 8fcc499c6f56f2a63fc05417f53d5ebdc746347e
Summary:
Release notes: none
I noticed that we weren't visiting the dependencies of temporalAlias values, nor were we visiting ReactElements. Visiting both seems to improve visitor performance locally for me on our bundle.
Closes https://github.com/facebook/prepack/pull/2108
Differential Revision: D8359160
Pulled By: trueadm
fbshipit-source-id: 1d9cc0f1855772a46721b1675649269628062113
Summary:
Release Notes: Full integration with Nuclide UI coming in next PR. This PR sets up a pipeline for passing configs to the debugger, and uses that pipeline to specify what level of CompilerDiagnostic the debugger should break at. The debugger's diagnostic check sits within the `handleError` function and occurs before Prepack takes its own action.
Current CLI experience:
<img width="1680" alt="dbg-cli-example" src="https://user-images.githubusercontent.com/15720289/41180054-bd45d39e-6b21-11e8-8a24-9626adab737f.png">
Closes https://github.com/facebook/prepack/pull/2088
Differential Revision: D8355227
Pulled By: caiismyname
fbshipit-source-id: 3ca457bcbb7697fd39073c3d2ddc88be92bfc662
Summary:
Release notes: none
Whilst working on adding React Native mocks, I ran into cases where context should be inlining but it wasn't. It now is inlining far better, with more test coverage. Furthermore, we now have a new config flag to pass to `__optimizeReactComponentTree`, in the case of `isRoot` to state that the tree is a root component tree (rather than a a branch of another tree).
Closes https://github.com/facebook/prepack/pull/2098
Differential Revision: D8348381
Pulled By: trueadm
fbshipit-source-id: 5e01bd77437e8bc3d1f22ff47d668897152203a0
Summary:
Turns out os.cpus() can return [] and undefined.
Additionally, it seems to me like any number below 4 could lead to an empty pool so I tried to address that too.
Reviewed By: NTillmann
Differential Revision: D8332896
fbshipit-source-id: 60e082a4f0d18e72f95b76732a50adeccb2d333d
Summary:
Release note: none
Effects objects are currently stateful: If you apply the effects it becomes an undo map and if you undo the effects, it becomes a redo map.
In most situations this is just fine since apply and undo are usually nicely paired. In some situations, however, we end up applying or undoing the same effects object twice in a row. This causes issues that are extremely hard to debug. I've tried to purge the code base of all such cases, but they just keep coming up. In essence, I am trying to maintain a very subtle invariant that is quite hard to check and I am failing.
I now give up on this invariant and am making Effects into an object that is immutable except when being constructed incrementally. This is the first step towards that.
Closes https://github.com/facebook/prepack/pull/2099
Differential Revision: D8334815
Pulled By: hermanventer
fbshipit-source-id: ab508ed6c6c42969c8fda842fa20b55f7773c2cc
Summary:
The code didn't work without it sometimes because conceptually all code outside of `getTrials` is an isolated "bundle". We can only access its internals through the `Root` reference. This fixes the tests to follow this constraint and removes the hack.
Closes https://github.com/facebook/prepack/pull/2102
Differential Revision: D8333634
Pulled By: gaearon
fbshipit-source-id: 1fb24751490042beae55a133f64351a9be9f903c
