@samitouri / QOS-React-1 / commits / 0ca9d20772

[compiler] Count loop reassignments as the enclosing scope reassigning the variable (#36732)

A counter initialized before a memo scope and incremented inside it (`a++` or `a = a + 1` in a `for` body) was emitted as a scope *dependency*, compared at its pre-loop value (constant every render) while the cache stored its post-loop value. The memo could never hit, so the scope recomputed on every render. Root cause: the phi-union rule in `InferReactiveScopeVariables.findDisjointMutableValues` only unioned a phi into the scope when the phi value was mutated after creation, which range-extension only does for object mutation. Primitive reassignments around a loop back-edge never extend ranges, so the counter's SSA versions stayed outside the scope and downstream dependency propagation classified the pre-loop read as a dep. The fix unions a phi with its operands and declaration when any operand is defined at or after the phi's block, i.e. the value is reassigned around a loop back-edge. This matches the shape the compiler already produced for non-primitive loop reassignment (`x = [...x, i]`). Implemented identically in the TypeScript compiler and the Rust port. Both `a++` and `a = a + 1` variants are pinned by fixtures; the first commit documents the previously-wrong codegen, the second fixes it (counter becomes a scope output, dep on `count` only). Corpus delta beyond the new fixtures is 4 fixtures, all strict improvements with byte-identical eval output: `for-in-statement-break`, `for-in-statement-continue`, `for-in-statement-type-inference` (loops previously re-ran every render, now memoized), and `sequence-expression` (two memo blocks collapse into one). Known limitation, unchanged from before: conditional reassignment in a loop (`for (...) { if (c) a++; }`) routes through a join phi that this rule cannot see; that shape behaves as it did before this change. Verification: TS snap 1806/1806, Rust snap 1806/1806, cargo workspace green, scoped TS-vs-Rust HIR parity harness green. Closes #34971

lauren committed Jun 17, 2026 at 15:54 UTC 0ca9d2077245a026f3b1430cede786a69d312434
10 files changed +238 -45
compiler/crates/react_compiler_inference/src/infer_reactive_scope_variables.rs
+20 -1
@@ -284,7 +284,26 @@ pub(crate) fn find_disjoint_mutable_values(
284 .map(|iid| func.instructions[iid.0 as usize].id)
285 .unwrap_or(block.terminal.evaluation_order());
286
287 - if phi_range.start.0 + 1 != phi_range.end.0 && phi_range.end > first_instr_id {
287 + let is_phi_mutated_after_creation = phi_range.start.0 + 1 != phi_range.end.0
288 + && phi_range.end > first_instr_id;
289 + // A phi operand defined at or after the phi's block is a loop
290 + // back-edge: the variable is reassigned within the loop (eg a
291 + // counter `a++` or `a = a + 1`). The reassignment must count as
292 + // the loop's scope reassigning the variable, so union the phi
293 + // with its operands and declaration. Otherwise the variable's
294 + // pre-loop value would become a dependency of the scope even
295 + // though the scope changes the value as it executes, making the
296 + // scope's dependencies unstable (the cached dependency would be
297 + // the post-loop value, which can never match the pre-loop value
298 + // compared at the top of the scope).
299 + let is_loop_carried_reassignment = !is_phi_mutated_after_creation
300 + && phi.operands.iter().any(|(_pred_id, operand)| {
301 + env.identifiers[operand.identifier.0 as usize]
302 + .mutable_range
303 + .start
304 + >= first_instr_id
305 + });
306 + if is_phi_mutated_after_creation || is_loop_carried_reassignment {
307 let mut operands = vec![phi_id];
308 if let Some(&decl_id) = declarations.get(&phi_decl_id) {
309 operands.push(decl_id);
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
+24 -5
@@ -25,7 +25,7 @@ import {
25 eachPatternOperand,
26 } from '../HIR/visitors';
27 import DisjointSet from '../Utils/DisjointSet';
28 -import {assertExhaustive} from '../Utils/utils';
28 +import {Iterable_some, assertExhaustive} from '../Utils/utils';
29
30 /*
31 * Note: this is the 1st of 4 passes that determine how to break a function into discrete
@@ -287,12 +287,31 @@ export function findDisjointMutableValues(
287 * are assigned to the same scope.
288 */
289 for (const phi of block.phis) {
290 - if (
290 + const firstInstructionIdOfBlock =
291 + block.instructions.at(0)?.id ?? block.terminal.id;
292 + const isPhiMutatedAfterCreation =
293 phi.place.identifier.mutableRange.start + 1 !==
294 phi.place.identifier.mutableRange.end &&
293 - phi.place.identifier.mutableRange.end >
294 - (block.instructions.at(0)?.id ?? block.terminal.id)
295 - ) {
295 + phi.place.identifier.mutableRange.end > firstInstructionIdOfBlock;
296 + /*
297 + * A phi operand defined at or after the phi's block is a loop back-edge:
298 + * the variable is reassigned within the loop (eg a counter `a++` or
299 + * `a = a + 1`). The reassignment must count as the loop's scope
300 + * reassigning the variable, so union the phi with its operands and
301 + * declaration. Otherwise the variable's pre-loop value would become a
302 + * dependency of the scope even though the scope changes the value as it
303 + * executes, making the scope's dependencies unstable (the cached
304 + * dependency would be the post-loop value, which can never match the
305 + * pre-loop value compared at the top of the scope).
306 + */
307 + const isLoopCarriedReassignment =
308 + !isPhiMutatedAfterCreation &&
309 + Iterable_some(
310 + phi.operands.values(),
311 + operand =>
312 + operand.identifier.mutableRange.start >= firstInstructionIdOfBlock,
313 + );
314 + if (isPhiMutatedAfterCreation || isLoopCarriedReassignment) {
315 const operands = [phi.place.identifier];
316 const declaration = declarations.get(
317 phi.place.identifier.declarationId,
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-in-statement-break.expect.md
+10 -12
@@ -29,21 +29,19 @@ import { c as _c } from "react/compiler-runtime";
29 function Component(props) {
30 const $ = _c(2);
31 let x;
32 - let t0;
32 if ($[0] !== props.value) {
34 - t0 = { ...props.value };
33 + const object = { ...props.value };
34 + for (const y in object) {
35 + if (y === "break") {
36 + break;
37 + }
38 +
39 + x = object[y];
40 + }
41 $[0] = props.value;
36 - $[1] = t0;
42 + $[1] = x;
43 } else {
38 - t0 = $[1];
39 - }
40 - const object = t0;
41 - for (const y in object) {
42 - if (y === "break") {
43 - break;
44 - }
45 -
46 - x = object[y];
44 + x = $[1];
45 }
46
47 return x;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-in-statement-continue.expect.md
+10 -12
@@ -38,21 +38,19 @@ import { c as _c } from "react/compiler-runtime";
38 function Component(props) {
39 const $ = _c(2);
40 let x;
41 - let t0;
41 if ($[0] !== props.value) {
43 - t0 = { ...props.value };
42 + const object = { ...props.value };
43 + for (const y in object) {
44 + if (y === "continue") {
45 + continue;
46 + }
47 +
48 + x = object[y];
49 + }
50 $[0] = props.value;
45 - $[1] = t0;
51 + $[1] = x;
52 } else {
47 - t0 = $[1];
48 - }
49 - const object = t0;
50 - for (const y in object) {
51 - if (y === "continue") {
52 - continue;
53 - }
54 -
55 - x = object[y];
53 + x = $[1];
54 }
55
56 return x;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-in-statement-type-inference.expect.md
+11 -4
@@ -25,14 +25,21 @@ export const FIXTURE_ENTRYPOINT = {
25 ## Code
26
27 ```javascript
28 -// @enablePreserveExistingMemoizationGuarantees:false
28 +import { c as _c } from "react/compiler-runtime"; // @enablePreserveExistingMemoizationGuarantees:false
29 const { identity, mutate } = require("shared-runtime");
30
31 function Component(props) {
32 + const $ = _c(2);
33 let x;
33 - const object = { ...props.value };
34 - for (const y in object) {
35 - x = y;
34 + if ($[0] !== props.value) {
35 + const object = { ...props.value };
36 + for (const y in object) {
37 + x = y;
38 + }
39 + $[0] = props.value;
40 + $[1] = x;
41 + } else {
42 + x = $[1];
43 }
44
45 mutate(x);
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-counter-memoization-assignment.expect.md new
+62
@@ -0,0 +1,62 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {Stringify} from 'shared-runtime';
6 +
7 +function Component({count}) {
8 + let a = 0;
9 + const items = [];
10 + for (let i = 0; i < count; i++) {
11 + a = a + 1;
12 + items.push(a);
13 + }
14 + return <Stringify items={items} a={a} />;
15 +}
16 +
17 +export const FIXTURE_ENTRYPOINT = {
18 + fn: Component,
19 + params: [{count: 2}],
20 + sequentialRenders: [{count: 2}, {count: 2}, {count: 3}],
21 +};
22 +
23 +```
24 +
25 +## Code
26 +
27 +```javascript
28 +import { c as _c } from "react/compiler-runtime";
29 +import { Stringify } from "shared-runtime";
30 +
31 +function Component(t0) {
32 + const $ = _c(2);
33 + const { count } = t0;
34 + let t1;
35 + if ($[0] !== count) {
36 + let a = 0;
37 + const items = [];
38 + for (let i = 0; i < count; i++) {
39 + a = a + 1;
40 + items.push(a);
41 + }
42 + t1 = <Stringify items={items} a={a} />;
43 + $[0] = count;
44 + $[1] = t1;
45 + } else {
46 + t1 = $[1];
47 + }
48 + return t1;
49 +}
50 +
51 +export const FIXTURE_ENTRYPOINT = {
52 + fn: Component,
53 + params: [{ count: 2 }],
54 + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }],
55 +};
56 +
57 +```
58 +
59 +### Eval output
60 +(kind: ok) <div>{"items":[1,2],"a":2}</div>
61 +<div>{"items":[1,2],"a":2}</div>
62 +<div>{"items":[1,2,3],"a":3}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-counter-memoization-assignment.js new
+17
@@ -0,0 +1,17 @@
1 +import {Stringify} from 'shared-runtime';
2 +
3 +function Component({count}) {
4 + let a = 0;
5 + const items = [];
6 + for (let i = 0; i < count; i++) {
7 + a = a + 1;
8 + items.push(a);
9 + }
10 + return <Stringify items={items} a={a} />;
11 +}
12 +
13 +export const FIXTURE_ENTRYPOINT = {
14 + fn: Component,
15 + params: [{count: 2}],
16 + sequentialRenders: [{count: 2}, {count: 2}, {count: 3}],
17 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-counter-memoization.expect.md new
+62
@@ -0,0 +1,62 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {Stringify} from 'shared-runtime';
6 +
7 +function Component({count}) {
8 + let a = 0;
9 + const items = [];
10 + for (let i = 0; i < count; i++) {
11 + a++;
12 + items.push(a);
13 + }
14 + return <Stringify items={items} a={a} />;
15 +}
16 +
17 +export const FIXTURE_ENTRYPOINT = {
18 + fn: Component,
19 + params: [{count: 2}],
20 + sequentialRenders: [{count: 2}, {count: 2}, {count: 3}],
21 +};
22 +
23 +```
24 +
25 +## Code
26 +
27 +```javascript
28 +import { c as _c } from "react/compiler-runtime";
29 +import { Stringify } from "shared-runtime";
30 +
31 +function Component(t0) {
32 + const $ = _c(2);
33 + const { count } = t0;
34 + let t1;
35 + if ($[0] !== count) {
36 + let a = 0;
37 + const items = [];
38 + for (let i = 0; i < count; i++) {
39 + a++;
40 + items.push(a);
41 + }
42 + t1 = <Stringify items={items} a={a} />;
43 + $[0] = count;
44 + $[1] = t1;
45 + } else {
46 + t1 = $[1];
47 + }
48 + return t1;
49 +}
50 +
51 +export const FIXTURE_ENTRYPOINT = {
52 + fn: Component,
53 + params: [{ count: 2 }],
54 + sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }],
55 +};
56 +
57 +```
58 +
59 +### Eval output
60 +(kind: ok) <div>{"items":[1,2],"a":2}</div>
61 +<div>{"items":[1,2],"a":2}</div>
62 +<div>{"items":[1,2,3],"a":3}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-counter-memoization.js new
+17
@@ -0,0 +1,17 @@
1 +import {Stringify} from 'shared-runtime';
2 +
3 +function Component({count}) {
4 + let a = 0;
5 + const items = [];
6 + for (let i = 0; i < count; i++) {
7 + a++;
8 + items.push(a);
9 + }
10 + return <Stringify items={items} a={a} />;
11 +}
12 +
13 +export const FIXTURE_ENTRYPOINT = {
14 + fn: Component,
15 + params: [{count: 2}],
16 + sequentialRenders: [{count: 2}, {count: 2}, {count: 3}],
17 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/sequence-expression.expect.md
+5 -11
@@ -19,22 +19,16 @@ function foo() {}
19 ```javascript
20 import { c as _c } from "react/compiler-runtime";
21 function sequence(props) {
22 - const $ = _c(2);
23 - let t0;
22 + const $ = _c(1);
23 + let x;
24 if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
25 - t0 = (Math.max(1, 2), foo());
26 - $[0] = t0;
27 - } else {
28 - t0 = $[0];
29 - }
30 - let x = t0;
31 - if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
25 + x = (Math.max(1, 2), foo());
26 while ((foo(), true)) {
27 x = (foo(), 2);
28 }
35 - $[1] = x;
29 + $[0] = x;
30 } else {
37 - x = $[1];
31 + x = $[0];
32 }
33
34 return x;