Summary:
Release notes: the side-effect detection system within pure scopes has been improved
This PR revamps the entire `evaluatePure` system. The old system had so many small issues with it was the cause of many edge-cases that were hard to patch up. Specifically, the PR makes the following changes:
- `realm.evaluatePure` -> `realm.evaluateWithPureScope` and now can only be used a single instance. Nesting further `realm.evaluateWithPureScope` calls will trigger an invariant and is strictly forbidden. Furthermore, it now only takes a single argument – the function you're wrapped around.
- All places that used pure scope wrappers have been updated to conditionally enable it depending if the `realm.isInPureScope()` returns `true` or `false`.
- `realm.evaluateFunctionForPureEffects` has been added, this works like `evaluateForEffects` except it requires a root function and a callback for side-effects. The callback for side-effects works like the old callback that was `evaluatePure`.
- `realm.evaluateFunctionForPureEffectsInGlobalEnv` has been added a convenience wrapper around `realm.evaluateFunctionForPureEffects`.
- When we leak bindings, we no longer set their value to `undefined` or `realm.intrinsics.undefined`. We now set the value to a special "leaked abstract", specifically – `realm.intrinsics.__leakedValue` – like `topValue` and `bottomValue`.
Unsurprisingly, this now fixes a host of bugs that existed before. Including fixes for https://github.com/facebook/prepack/issues/2598, https://github.com/facebook/prepack/issues/2579, https://github.com/facebook/prepack/issues/2446, https://github.com/facebook/prepack/issues/2599 and probably many other issues too.
The logic for detection of side-effects works very differently from before but after speaking to sebmarkbage last week, he pointed me in this direction to track side-effects rather than how we did it before. We now find side-effects from a given set of effects, rather than in an ad-hoc manor as mutations occur on objects/bindings. This PR requires https://github.com/facebook/prepack/pull/2596 as a dependency as it re-uses the logic.
Closes https://github.com/facebook/prepack/pull/2587. Fixes https://github.com/facebook/prepack/issues/2598. Fixes https://github.com/facebook/prepack/issues/2579. Fixes https://github.com/facebook/prepack/issues/2446. Fixes https://github.com/facebook/prepack/issues/2599.
Pull Request resolved: https://github.com/facebook/prepack/pull/2600
Differential Revision: D10368159
Pulled By: trueadm
fbshipit-source-id: ded248f5cfd8648913cae9b9c697d723a82c59ab
Summary:
Release notes:
This adds a `reactFailOnUnsupportedSideEffects` flag for React internal testing with `debug-fb-www` and ignores `try/catch` PP0021 errors for internal testing too. These are intended to be temporary changes purely for internal FB testing.
Pull Request resolved: https://github.com/facebook/prepack/pull/2531
Differential Revision: D9656578
Pulled By: trueadm
fbshipit-source-id: 9339dd8245e5a03d567ca2fd8e24e90152f1e22b
Summary:
Release notes: adds a `arrayNestedOptimizedFunctionsEnabled` flag to enable nested optimized functions derived from Array.prototype methods (like `map`) and Array.from
This PR puts the existing (unstable) work for Array prototype methods behind a flag. The flag is enabled by default in React and serializer tests.
Pull Request resolved: https://github.com/facebook/prepack/pull/2404
Differential Revision: D9272747
Pulled By: trueadm
fbshipit-source-id: d7e53656a12cd6cff680a9ef0e2580a93d56e34e
Summary:
Release notes: upgrades Prepack to use Babel 7.0.0-beta.53
This is a big PR that updates all of Prepack to Babel 7. Babylon is now `babel/parser` and pretty much all of the the previous Babel packages are now located in scoped packages. I had to make a bunch of changes around Jest/Flow/Webpack to get this all working. The build times of building Prepack itself seem considerably faster (easily twice as fast locally). I followed most of the Babel 6 -> 7 upgrade guide from the Babel site in terms of changing nodes and type definitions to match the new ones.
Pull Request resolved: https://github.com/facebook/prepack/pull/2256
Differential Revision: D8850583
Pulled By: trueadm
fbshipit-source-id: 2d2aaec25c6a1ccd1ec0c08c5e7e2a71f78ac2d8
Summary:
Release notes: None
Tweaking some behaviors to help with Instant Render automation.
This leaves behind some TODOs in the code, but unblock me for now.
Closes https://github.com/facebook/prepack/pull/2178
Differential Revision: D8689796
Pulled By: NTillmann
fbshipit-source-id: f919a8b6bbfb63513bb9467027676e0ba7d4230e
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 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
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: 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: 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:
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: 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:
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
This PR adds nested optimized function support and bug fixes around them to the React reconciler.
The tests added that fail have been skipped for now until we have the ability to properly serialize nested optimized functions.
Closes https://github.com/facebook/prepack/pull/1614
Differential Revision: D7366102
Pulled By: trueadm
fbshipit-source-id: c18c5a470a31b905a76155e006767f8ec7a73afa
Summary:
Release notes: none
For debug-fb-ww we can recover from key and ref issues on ReactElements so we can evaluate more.
Closes https://github.com/facebook/prepack/pull/1629
Differential Revision: D7352493
Pulled By: trueadm
fbshipit-source-id: ecafec23d32c5cefbe86a8ec7100d3c28d21bb87
Summary:
Release notes: none
When working with lots of component roots, having the build fatal hard on a root bail-out out is just added noise and stops the workflow. This PR changes this logic so they don't hard fail and instead just bail out like branch trees do already. The information for the bail-out still gets logged.
Closes https://github.com/facebook/prepack/pull/1628
Differential Revision: D7350189
Pulled By: trueadm
fbshipit-source-id: e8424a5f39a9bb536838bea29111ed74d1584196
Summary:
Release notes: Removing --abstractEffectsInAdditionalFunctions option
It is now the new default, as nothing seems to depend on the old behavior.
Closes https://github.com/facebook/prepack/pull/1623
Differential Revision: D7342083
Pulled By: NTillmann
fbshipit-source-id: c02d13bddd0ff3d8a113a6a5a83c1bf35c96ba4d
Summary:
Release note: Add source annotations to satisfy latest version of Flow
Flow now complains about missing properties in more situations.
Closes https://github.com/facebook/prepack/pull/1612
Differential Revision: D7324360
Pulled By: hermanventer
fbshipit-source-id: 0aa40f2442a13c87258179f2151864cf9c921215
Summary:
I presume this was the intended meaning. Not sure how it's different from the stack though.
Closes https://github.com/facebook/prepack/pull/1611
Reviewed By: trueadm
Differential Revision: D7321041
Pulled By: gaearon
fbshipit-source-id: c1fee2b6ba0113a04d6e93c2c7e5d9b2241f14ee
Summary:
Release notes: adds an abstract value limiting counter
Following some discussion I had the other night with hermanventer in regards to having a counter for Prepack abstract logic computation, so rather than hang the compilation process, we could throw and recover using the pure logic.
This PR adds a "k-limiting" counter to `AbstractValue`'s as a method `_checkAbstractValueImpliesCounter `. When we try and resolve abstract conditions on a single AbstractValue and we reach the limit, a `CompilerDiagnostic` along with a `FatalError` gets thrown. Locally this works well for the React compiler input.
I only call `_checkAbstractValueImpliesCounter ` from `implies` right now and that might not be the best place, maybe it should be checked in other places too, but `implies` seemed the good place as it was in all stackframes when debugging.
This also adds a new option called `abstractValueImpliesCounter ` which is based as an option to Realm. By default this feature is not turned on when the option `abstractValueImpliesCounter ` value is `0` (which it has been set to). Maybe we should enable this by default and set it to a sensible level?
**Note:**
I tried using the `timeout` feature, but it didn't work and wasn't too reliable in general because it would always fire in places where I didn't care for "time taken" but rather the depth on a certain feature of computation – in this case, abstract logic simplification.
Enabling the timeout causes many other parts of compilation to break unless I explicitly set the timeout to `0` through all the React reconciliation work, which kind of defeats the purpose of it (the React reconciler work can take minutes to deal with on very large, expensive trees, and this is totally expected).
Furthermore, we should avoid using non-deterministic features like `feature` in such cases as they can skew test results.
Closes https://github.com/facebook/prepack/pull/1607
Differential Revision: D7312616
Pulled By: trueadm
fbshipit-source-id: 8763df09b3182af4a496dec42d507aff1bb2cc8d
Summary:
Release note: none
Debugging is more painful with debug-fb-ww than it needs to be, because the AST that goes into the abstract interpreter does not have source locations.
This little change fixes that. Please be so kind as to bless it.
Closes https://github.com/facebook/prepack/pull/1604
Differential Revision: D7305670
Pulled By: hermanventer
fbshipit-source-id: fea20e987278ba45be79a29c7d0e84bb4b20ddc4
Summary:
Release notes: none
Adds a few more options on to the linter.
Closes https://github.com/facebook/prepack/pull/1601
Differential Revision: D7294853
Pulled By: trueadm
fbshipit-source-id: 4ec2dd821afd63bcb1aabfbcad2ce33a1af8c06a
Summary:
Release notes: none
Validate the output of running `debug-fb-www` and check there are no undefined vars using eslint.
Closes https://github.com/facebook/prepack/pull/1597
Differential Revision: D7287926
Pulled By: trueadm
fbshipit-source-id: 6908f44b62ef251457495e6c2e69dc5e0c35aeb0
Summary:
Release notes: none
More improvements the React reconciler so it gives better output and scan/tracks for more trees.
Closes https://github.com/facebook/prepack/pull/1539
Differential Revision: D7186071
Pulled By: trueadm
fbshipit-source-id: 8cf3c08e23a6d3b386917e58371b4f021a537d7e
Summary:
Release notes: none
Various fixes to get the UFI evaluating without invariants and other issues. Also added `componentsEvaluated` statistics.
Closes https://github.com/facebook/prepack/pull/1535
Differential Revision: D7172478
Pulled By: trueadm
fbshipit-source-id: 683a1039f22241261e7a5e216ec614b21ff57a92
Summary:
Release notes: none
We spam the console when running `test-react` and many of the errors are not errors but just noise. Interestingly this also brought about one test that genuinely is failing but before was bringing up an invalid bail-out with `dynamic-props.js`. I'll fix that as a follow up as it's unrelated to this PRs goal.
Closes https://github.com/facebook/prepack/pull/1529
Differential Revision: D7149640
Pulled By: trueadm
fbshipit-source-id: e3268bc92c62726a5686b1acfcfff4128b69f6cd
Summary:
Release notes: none
This generates a tree graph of how all the components in the React reconciliation process get handled. It will massively help debugging what did or did not get inlined/evaluated. The data is also added to snapshots to further improve regression testing too.
Closes https://github.com/facebook/prepack/pull/1509
Differential Revision: D7115923
Pulled By: trueadm
fbshipit-source-id: 4d17dccc52b324c9a0a59a5864066345ae7ec097
