Traverse program after running compiler transform to find untransformed
references to compiler features (e.g. `inferEffectDeps`, `fire`).
Hard error to fail the babel pipeline when the compiler fails to
transform these features to give predictable runtime semantics.
Untransformed calls to functions like `fire` will throw at runtime
anyways, so let's fail the build to catch these earlier.
Note that with this fails the build *regardless of panicThreshold*
Removes `EnvironmentConfig.enableMinimalTransformsForRetry` in favor of
`run` parameters. This is a minimal difference but lets us explicitly
opt out certain compiler passes based on mode parameters, instead of
environment configurations
Retry flags don't really make sense to have in `EnvironmentConfig`
anyways as the config is user-facing API, while retrying is a compiler
implementation detail.
(per @josephsavona's feedback
https://github.com/facebook/react/pull/32164#issuecomment-2608616479)
> Re the "hacky" framing of this in the PR title: I think this is fine.
I can see having something like a compilation or output mode that we use
when running the pipeline. Rather than changing environment settings
when we re-run, various passes could take effect based on the
combination of the mode + env flags. The modes might be:
>
> * Full: transform, validate, memoize. This is the default today.
> * Transform: Along the lines of the backup mode in this PR. Only
applies transforms that do not require following the rules of React,
like `fire()`.
> * Validate: This could be used for ESLint.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32511).
* #32512
* __->__ #32511
This change fixes a coverage hole in rolling out with `gating`. Prior to
this PR, configuring `gating` causes React Compiler to bail out of
optimizing some functions.
This means that it's not entirely safe to cutover from `gating` enabled
for all users (i.e. rolled out 100%) to removing the `gating` config
altogether, as new functions may be opted into compilation when they
stop bailing out due to gating-specific logic.
This is technically slightly slower due to the additional function
indirection. An alternative approach is to recommend running a codemod
to insert `use no memo`s on currently-bailing out functions before
removing the`gating` config.
---
Tested [internally](
https://fburl.com/diff/q982ovua) by enabling on a page that previously
had a few hundred bailouts due to gating + hoisted function declarations
and (1) clicking around locally and (2) running a bunch of e2e tests
Reduce false positive bailouts by using the same
`isReferencedIdentifier` logic that the compiler also uses for
determining context variables and a function's own hoisted declarations.
Details:
Previously, we counted every babel identifier as a reference. This is
problematic because babel counts most string symbols as an identifier.
```js
print(x); // x is an identifier as expected
obj.x // x is.. also an identifier here
{x: 2} // x is also an identifier here
```
This PR adds a check for `isReferencedIdentifier`. Note that only
non-lval
references pass this check. This should be fine as we don't need to
hoist function declarations before writes to the same lvalue (which
should error in strict mode anyways)
```js
print(x); // isReferencedIdentifier(x) -> true
obj.x // isReferencedIdentifier(x) -> false
{x: 2} // isReferencedIdentifier(x) -> false
x = 2 // isReferencedIdentifier(x) -> false
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32596).
* __->__ #32596
* #32595
* #32594
* #32593
* #32522
* #32521
Expand type inference to infer mixedReadOnly types for numeric and
computed property accesses.
```js
function Component({idx})
const data = useFragment(...)
// we want to type `posts` correctly as Array
const posts = data.viewers[idx].posts.slice(0, 5);
// ...
}
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32593).
* #32596
* #32595
* #32594
* __->__ #32593
* #32522
* #32521
Summary: Correctly supports React.useEffect when React is
imported as `import * as React from 'react'`
(as well as other namespaces as specified in the config).
Test fixtures testing different compiler features (e.g. non-auto
memoization) should live in separate directories.
Remove bug-prefixed fixtures that have since been fixed
Add test evaluator export to more fixtures
Prior to this PR, our HIR represented property access with numeric
literals (e.g. `myVar[0]`) as ComputedLoads. This means that they were
subject to some deopts (most notably, not being easily dedupable /
hoistable as dependencies).
Now, `PropertyLoad`, `PropertyStore`, etc reference numeric and string
literals (although not yet string literals that aren't valid babel
identifiers). The difference between PropertyLoad and ComputedLoad is
fuzzy now (maybe we should rename these).
- PropertyLoad: property keys are string and numeric literals, only when
the string literals are valid babel identifiers
- ComputedLoad: non-valid babel identifier string literals (rare) and
other non-literal expressions
The biggest feature from this PR is that it trivially enables
array-indicing expressions as dependencies. The compiler can also
specify global and imported types for arrays (e.g. return value of
`useState`)
I'm happy to close this if it complicates more than it helps --
alternative options are to entirely rely on instruction reordering-based
approaches like ReactiveGraphIR or make dependency-specific parsing +
hoisting logic more robust.
LoweredFunction dependencies were exclusively used for dependency
extraction (in `propagateScopeDeps`). Now that we have a
`propagateScopeDepsHIR` that recursively traverses into nested
functions, we can delete `dependencies` and their associated synthetic
`LoadLocal`/`PropertyLoad` instructions.
[Internal snapshot
diff](https://www.internalfb.com/phabricator/paste/view/P1716950202) for
this change shows ~.2% of files changed. I [read through ~60 of the
changed
files](https://www.internalfb.com/phabricator/paste/view/P1733074307)
- most changes are due to better outlining (due to better DCE)
- a few changes in memo inference are due to changed ordering
```
// source
arr.map(() => contextVar.inner);
// previous instructions
$0 = LoadLocal arr
$1 = $0.map
// Below instructions are synthetic
$2 = LoadLocal contextVar
$3 = $2.inner
$4 = Function deps=$3 context=contextVar {
...
}
```
- a few changes are effectively bugfixes (see
`aliased-nested-scope-fn-expr`)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32096).
* #32099
* #32286
* #32104
* #32098
* #32097
* __->__ #32096
Hacky retry pipeline for when transforming `fire(...)` calls encounters
validation, todo, or memoization invariant bailouts. Would love feedback
on how we implement this to be extensible to other compiler
non-memoization features (e.g. inlineJSX)
Some observations:
- Compiler "front-end" passes (e.g. lower, type, effect, and mutability
inferences) should be shared for all compiler features -- memo and
otherwise
- Many passes (anything dealing with reactive scope ranges, scope blocks
/ dependencies, and optimizations such as ReactiveIR #31974) can be left
out of the retry pipeline. This PR hackily skips memoization features by
removing reactive scope creation, but we probably should restructure the
pipeline to skip these entirely on a retry
- We should maintain a canonical set of "validation flags"
Note the newly added fixtures are prefixed with `bailout-...` when the
retry fire pipeline is used. These fixture outputs contain correctly
inserted `useFire` calls and no memoization.
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.
Before submitting a pull request, please make sure the following is
done:
1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
10. If you haven't already, complete the CLA.
Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->
## Summary
Our [LlamaIndex](https://www.llamaindex.ai/) Product is blocked by this
bug
Fixes: https://github.com/facebook/react/issues/32137
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
## How did you test this change?
<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
If you leave this empty, your PR will very likely be closed.
-->
For now we just reject all calls of impure functions, and the validation
is off by default. Going forward we can make this more precise and only
reject impure functions called during render.
Note that I was intentionally imprecise in the return type of these
functions in order to avoid changing output of existing code. We lie to
the compiler and say that Date.now, performance.now, and Math.random
return unknown mutable objects rather than primitives. Once the
validation is complete and vetted we can switch this to be more precise.
- Adds @compilationMode(all|infer|syntax|annotation) and
@panicMode(none) directives. This is now shared with our test infra
- Playground still defaults to `infer` mode while tests default to `all`
mode
- See added fixture tests
Traverse the compiled functions to ensure there are no lingering fires
and that all
fire calls are inside an effect lambda.
Also corrects the import to import from the compiler runtime instead
--
This is the diff with the meaningful changes. The approach is:
1. Collect fire callees and remove fire() calls, create a new binding
for the useFire result
2. Update LoadLocals for captured callees to point to the useFire result
3. Update function context to reference useFire results
4. Insert useFire calls after getting to the component scope
This approach aims to minimize the amount of new bindings we introduce
for the function expressions
to minimize bookkeeping for dependency arrays. We keep all of the
LoadLocals leading up to function
calls as they are and insert new instructions to load the originally
captured function, call useFire,
and store the result in a new promoted temporary. The lvalues that
referenced the original callee are
changed to point to the new useFire result.
This is the minimal diff to implement the expected behavior (up to
importing the useFire call, next diff)
and further stacked diffs implement error handling. The rules for fire
are:
1. If you use fire for a callee in the effect once you must use it for
every time you call it in that effect
2. You can only use fire in a useEffect lambda/functions defined inside
the useEffect lambda
There is still more work to do here, like updating the effect dependency
array and handling object methods
--
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31796).
* #31811
* #31798
* #31797
* __->__ #31796
We report a false positive for the combination of a ref-accessing
function placed inside an array which is they type-cast. Here we teach
ref validation about type casts. I also tried other variants like
`return ref as const` but those already worked.
Closes#31864
Add shape / type for global Object.keys. This is useful because
- it has an Effect.Read (not an Effect.Capture) as it cannot alias its
argument.
- Object.keys return an array
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31583).
* __->__ #31583
* #31582
We previously didn't track context variables in the hoistable values
sidemap of `propagateScopeDependencies`. This was overly conservative as
we *do* track the mutable range of context variables, and it is safe to
hoist accesses to context variables after their last direct / aliased
maybe-assignment.
```js
function Component({value}) {
// start of mutable range for `x`
let x = DEFAULT;
const setX = () => x = value;
const aliasedSet = maybeAlias(setX);
maybeCall(aliasedSet);
// end of mutable range for `x`
// here, we should be able to take x (and property reads
// off of x) as dependencies
return <Jsx value={x} />
}
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31582).
* #31583
* __->__ #31582
Compiler playground now runs the entire program through
`babel-plugin-react-compiler` instead of a custom pipeline which
previously duplicated function inference logic from `Program.ts`. In
addition, the playground output reflects the tranformed file (instead of
a "virtual file" of manually concatenated functions).
This helps with the following:
- Reduce potential discrepencies between playground and babel plugin
behavior. See attached fixture output for an example where we previously
diverged.
- Let playground users see compiler-inserted imports (e.g. `_c` or
`useFire`)
This also helps us repurpose playground into a more general tool for
compiler-users instead of just for compiler engineers.
- imports and other functions are preserved.
We differentiate between imports and globals in many cases (e.g.
`inferEffectDeps`), so it may be misleading to omit imports in printed
output
- playground now shows other program-changing behavior like position of
outlined functions and hoisted declarations
- emitted compiled functions do not need synthetic names
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31774).
* #31809
* __->__ #31774
When supporting ref as prop in
https://github.com/facebook/react/pull/31558, I missed fixing the
optimization to pass a spread-props-only props object in without an
additional object copy. In the case that we have only a ref along with a
spread, we cannot return only the spread object. This results in
dropping the ref.
In this example
```javascript
<Foo ref={ref} {...props} />
```
The bugged output is:
```javascript
{
// ...
props: props
}
```
With this change we now get the correct output:
```javascript
{
// ...
props: {ref: ref, ...props}
}
```
Any LoadGlobal in the "infer deps" position can safely use an empty dep
array. Globals have no reactive deps!
I just keep messing up sapling. This is the revised version of #31662
Adds `target: 'donotuse_meta_internal'`, which inserts useMemoCache
imports directly from `react`. Note that this is only valid for Meta
bundles, as others do not [re-export the `c`
function](https://github.com/facebook/react/blob/5b0ef217ef32333a8e56f39be04327c89efa346f/packages/react/index.fb.js#L68-L70).
```js
// target=donotuse_meta_internal
import {c as _c} from 'react';
// target=19
import {c as _c} from 'react/compiler-runtime';
// target=17,18
import {c as _c} from 'react-compiler-runtime';
```
Meta is a bit special in that react runtime and compiler are guaranteed
to be up-to-date and compatible. It also has its own bundling and module
resolution logic, which makes importing from `react/compiler-runtime`
tricky.
I'm also fine with implementing the alternative which adds an internal
stub for `react-compiler-runtime` and
[bundles](https://github.com/facebook/react/blob/5b0ef217ef32333a8e56f39be04327c89efa346f/scripts/rollup/bundles.js#L120)
the runtime for internal builds.
Adds a way to configure how we insert deps for experimental purposes.
```
[
{
module: 'react',
imported: 'useEffect',
numRequiredArgs: 1,
},
{
module: 'MyExperimentalEffectHooks',
imported: 'useExperimentalEffect',
numRequiredArgs: 2,
},
]
```
would insert dependencies for calls of `useEffect` imported from `react`
if they have 1 argument and calls of useExperimentalEffect` from
`MyExperimentalEffectHooks` if they have 2 arguments. The pushed dep
array is appended to the arg list.
We didn't originally support holes within array patterns, so DCE was
only able to prune unused items from the end of an array pattern. Now
that we support holes we can replace any unused item with a hole, and
then just prune the items to the last identifier/spread entry.
Note: this was motivated by finding useState where either the state or
setState go unused — both are strong indications that you're violating
the rules in some way. By DCE-ing the unused portions of the useState
destructuring we can easily check if you're ignoring either value.
closes#31603
This is a redo of that PR not using ghstack
Dimitri POSTOLOVandGitHub
mofeiZandGitHub
Jordan BrownandGitHub
inottnandGitHub
Alex YangandGitHub
Joseph SavonaandGitHub
laurenandGitHub
Jack PopeandGitHub
Mike Vitousek
Jordan BrownandGitHub