Ensures that effects are well-formed with respect to the rules:
* For a given instruction, each place is only initialized once (w one of
Create, CreateFrom, Assign)
* Ensures that Alias targets are already initialized within the same
instruction (should have a Create before them)
* Preserves Create and similar instructions
* Avoids duplicate instructions when inferring effects of function
expressions
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33558).
* #33571
* __->__ #33558
* #33547
Adds some typed helpers to represent aliasing, assign, capture,
createfrom, and mutate effects along with representative runtime
behavior, and then adds tests to demonstrate that we model
capture->createfrom and createfrom->capture correctly.
There is one case (createfrom->capture in a lambda) where we infer a
less precise effect, but in the more conservative direction (we include
more code/deps than necesssary rather than fewer).
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33543).
* #33571
* #33558
* #33547
* __->__ #33543
Now that we have support for defining aliasing signatures in
moduleTypeProvider, which uses string names for
receiver/args/returns/etc, we can reuse that same form for builtin
declarations. The declarations are written in the unparsed form and than
parsed/validated when registered (in the addFunction/addHook call).
This also required flushing out configs/schemas for more effect types.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33530).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* __->__ #33530
In comparing compilation output of the old/new inference models I found
this case (heavily distilled into a fixture). Roughly speaking the
scenario is:
* Create a mutable object `x`
* Extract part of that object and pass it to a hook/jsx so that _part_
becomes frozen
* Mutate `x`, even indirectly.
In the old model we can still independently memoize the value from the
middle step, since we assume that part of the larger value is not
changing. In the new model, the mutation from the later step effectively
overrides the freeze effect in step 2, and considers the value to have
changed later anyway.
We've already rolled out and vetted the previous behavior, confirming
that the heuristic of "that part of the mutable object is fozen now" is
generally safe. I'll fix in a follow-up.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33522).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* __->__ #33522
* #33518
The previous error for hoisting violations pointed only to the variable
declaration, but didn't show where the value was accessed before that
declaration. We now track where each hoisted variable is first accessed
and report two errors, one for the reference and one for the
declaration. When we improve our diagnostic infra to support reporting
errors at multiple locations we can merge these into a single conceptual
error.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33514).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* __->__ #33514
* #33573
The previous error message was generic, because the old style function
signature didn't support a way to specify a reason alongside a freeze
effect. This meant we could only say why a value was frozen for
instructions, but not hooks which use function signatures. By defining a
new aliasing signature for custom hooks we can specify a reason and
provide a better error message.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33513).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* __->__ #33513
AnalyzeFunctions had logic to reset the mutable ranges of context
variables after visiting inner function expressions. However, there was
a bug in that logic: InferReactiveScopeVariables makes all the
identifiers in a scope point to the same mutable range instance. That
meant that it was possible for a later function expression to indirectly
cause an earlier function expressions' context variables to get a
non-zero mutable range.
The fix is to not just reset start/end of context var ranges, but assign
a new range instance. Thanks for the help on debugging, @mofeiz!
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33500).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* #33513
* #33512
* #33504
* __->__ #33500
* #33497
* #33496
Squashed, review-friendly version of the stack from
https://github.com/facebook/react/pull/33488.
This is new version of our mutability and inference model, designed to
replace the core algorithm for determining the sets of instructions
involved in constructing a given value or set of values. The new model
replaces InferReferenceEffects, InferMutableRanges (and all of its
subcomponents), and parts of AnalyzeFunctions. The new model does not
use per-Place effect values, but in order to make this drop-in the end
_result_ of the inference adds these per-Place effects.
I'll write up a larger document on the model, first i'm doing some
housekeeping to rebase the PR.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33494).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* #33513
* #33512
* #33504
* #33500
* #33497
* #33496
* #33495
* __->__ #33494
* #33572
This bug was reported via our wg and appears to only affect values
created as a ref.
Currently, postfix operators used in a callback gets compiled to:
```js
modalId.current = modalId.current + 1; // 1
const id = modalId.current; // 1
return id;
```
which is semantically incorrect. The postfix increment operator should
return the value before incrementing. In other words something like this
should have been compiled instead:
```js
const id = modalId.current; // 0
modalId.current = modalId.current + 1; // 1
return id;
```
This bug does not trigger when the incremented value is a plain
primitive, instead there is a TODO bailout.
## Summary
This tool leverages DevTools to get the component tree from the
currently open React App. This gives realtime information to agents
about the state of the app.
## How did you test this change?
Tested integration with Claude Desktop
This is a babel bug + edge case.
Babel compact mode produces invalid JavaScript (i.e. parse error) when
given a `NumericLiteral` with a negative value.
See https://codesandbox.io/p/devbox/5d47fr for repro.
## Summary
While investigating the root cause of #33208, I noticed a clear typo for
one of the validation files.
## How did you test this change?
Inside `/react/compiler/packages/babel-plugin-react-compiler` I ran the
test script successfully:
<img width="415" alt="Screenshot at May 22 16-43-06"
src="https://github.com/user-attachments/assets/3fe8c5e1-37ce-4a31-b35e-7e323e57cd9d"
/>
Inferred effect dependencies now include optional chains.
This is a temporary solution while
https://github.com/facebook/react/pull/32099 and its followups are
worked on. Ideally, we should model reactive scope dependencies in the
IR similarly to `ComputeIR` -- dependencies should be hoisted and all
references rewritten to use the hoisted dependencies.
`
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33326).
* __->__ #33326
* #33325
* #32286
React Compiler's program traversal logic is pretty lengthy and complex
as we've added a lot of features piecemeal. `compileProgram` is 300+
lines long and has confusing control flow (defining helpers inline,
invoking visitors, mutating-asts-while-iterating, mutating global
`ALREADY_COMPILED` state).
- Moved more stuff to `ProgramContext`
- Separated `compileProgram` into a bunch of helpers
Tested by syncing this stack to a Meta codebase and observing no
compilation output changes (D74487851, P1806855669, P1806855379)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33147).
* #33149
* #33148
* __->__ #33147
New take on #29716
## Summary
Template literals consisting entirely of constant values will be inlined
to a string literal, effectively replacing the backticks with a double
quote.
This is done primarily to make the resulting instruction a string
literal, so it can be processed further in constant propatation. So this
is now correctly simplified to `true`:
```js
`` === "" // now true
`a${1}` === "a1" // now true
```
If a template string literal can only partially be comptime-evaluated,
it is not that useful for dead code elimination or further constant
folding steps and thus, is left as-is in that case. Same is true if the
literal contains an array, object, symbol or function.
## How did you test this change?
See added tests.
(Almost) all pragmas are now one of the following:
- `@...TestOnly`: custom pragma for test fixtures
- `@<configName>` | `@<configName>:true`: enables with either true or a
default enabled value
- `@<configName>:<json value>`
## Summary
`-constant` is represented as a `UnaryExpression` node that is currently
not part of constant folding. If the operand is a constant number, the
node is folded to `constant * -1`. This also coerces `-0` to `0`,
resulting in `0 === -0` being folded to `true`.
## How did you test this change?
See attached tests
Multiple things here:
- Improve the mean calculation for metrics so we don't report 0 when
web-vitals fail to be retrieved
- improve ui chaos monkey to use puppeteer APIs since only those trigger
INP/CLS metrics since we need emulated mouse clicks
- Add logic to navigate to a temp page after render since some
web-vitals metrics are only calculated when the page is backgrounded
- Some readability improvements
Note that bailing out adds false positives for hoisted functions whose
only references are within other functions. For example, this rewrite
would be safe.
```js
// source program
function foo() {
return bar();
}
function bar() {
return 42;
}
// compiler output
let bar;
if (/* deps changed */) {
function foo() {
return bar();
}
bar = function bar() {
return 42;
}
}
```
These false positives are difficult to detect because any maybe-call of
foo before the definition of bar would be invalid.
Instead of bailing out, we should rewrite hoisted function declarations
to the following form.
```js
let bar$0;
if (/* deps changed */) {
// All references within the declaring memo block
// or before the function declaration should use
// the original identifier `bar`
function foo() {
return bar();
}
function bar() {
return 42;
}
bar$0 = bar;
}
// All references after the declaring memo block
// or after the function declaration should use
// the rewritten declaration `bar$0`
```
This revisits a validation I built a while ago, trying to make it more strict this time to ensure that it's high-signal.
We detect function expressions which are *known* mutable — they definitely can modify a variable defined outside of the function expression itself (modulo control flow). This uses types to look for known Store and Mutate effects only, and disregards mutations of effects. Any such function passed to a location with a Freeze effect is reported as a validation error.
This is behind a flag and disabled by default. If folks agree this makes sense to revisit, i'll test out internally and we can consider enabling by default.
ghstack-source-id: 075a731444
Pull Request resolved: https://github.com/facebook/react/pull/33079