[compiler] Prevent innaccurate derivation recording on FunctionExpressions on no-derived-computation-in-effects (#35173)
Summary: The operands of a function expression are the elements passed as context. This means that it doesn't make sense to record mutations for them. The relevant mutations will happen in the function body, so we need to prevent FunctionExpression type instruction from running the logic for effect mutations. This was also causing some values to depend on themselves in some cases triggering an infinite loop. Also added n invariant to prevent this issue Test Plan: Added fixture test --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35173). * #35174 * __->__ #35173
Jorge Cabiedes committed
Nov 20, 2025 at 10:44 UTC
7ee974de927e9bbe10a44441ab49bafd9f5467a2
3 files changed
+184
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts
+37
@@ -52,6 +52,8 @@ type ValidationContext = {
52
readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
53
};
54
55
+const MAX_FIXPOINT_ITERATIONS = 100;
56
+
57
class DerivationCache {
58
hasChanges: boolean = false;
59
cache: Map<IdentifierId, DerivationMetadata> = new Map();
@@ -224,6 +226,7 @@ export function validateNoDerivedComputationsInEffects_exp(
226
}
227
228
let isFirstPass = true;
229
+ let iterationCount = 0;
230
do {
231
context.derivationCache.takeSnapshot();
232
@@ -236,6 +239,19 @@ export function validateNoDerivedComputationsInEffects_exp(
239
240
context.derivationCache.checkForChanges();
241
isFirstPass = false;
242
+ iterationCount++;
243
+ CompilerError.invariant(iterationCount < MAX_FIXPOINT_ITERATIONS, {
244
+ reason:
245
+ '[ValidateNoDerivedComputationsInEffects] Fixpoint iteration failed to converge.',
246
+ description: `Fixpoint iteration exceeded ${MAX_FIXPOINT_ITERATIONS} iterations while tracking derivations. This suggests a cyclic dependency in the derivation cache.`,
247
+ details: [
248
+ {
249
+ kind: 'error',
250
+ loc: fn.loc,
251
+ message: `Exceeded ${MAX_FIXPOINT_ITERATIONS} iterations in ValidateNoDerivedComputationsInEffects`,
252
+ },
253
+ ],
254
+ });
255
} while (context.derivationCache.snapshot());
256
257
for (const [, effect] of effectsCache) {
@@ -422,6 +438,14 @@ function recordInstructionDerivations(
438
);
439
}
440
441
+ if (value.kind === 'FunctionExpression') {
442
+ /*
443
+ * We don't want to record effect mutations of FunctionExpressions the mutations will happen in the
444
+ * function body and we will record them there.
445
+ */
446
+ return;
447
+ }
448
+
449
for (const operand of eachInstructionOperand(instr)) {
450
switch (operand.effect) {
451
case Effect.Capture:
@@ -512,6 +536,19 @@ function buildTreeNode(
536
537
const namedSiblings: Set<string> = new Set();
538
for (const childId of sourceMetadata.sourcesIds) {
539
+ CompilerError.invariant(childId !== sourceId, {
540
+ reason:
541
+ 'Unexpected self-reference: a value should not have itself as a source',
542
+ description: null,
543
+ details: [
544
+ {
545
+ kind: 'error',
546
+ loc: sourceMetadata.place.loc,
547
+ message: null,
548
+ },
549
+ ],
550
+ });
551
+
552
const childNodes = buildTreeNode(
553
childId,
554
context,
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/function-expression-mutation-edge-case.expect.md
new
+115
@@ -0,0 +1,115 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
6
+
7
+function Component() {
8
+ const [foo, setFoo] = useState({});
9
+ const [bar, setBar] = useState(new Set());
10
+
11
+ /*
12
+ * isChanged is considered context of the effect's function expression,
13
+ * if we don't bail out of effect mutation derivation tracking, isChanged
14
+ * will inherit the sources of the effect's function expression.
15
+ *
16
+ * This is innacurate and with the multiple passes ends up causing an infinite loop.
17
+ */
18
+ useEffect(() => {
19
+ let isChanged = false;
20
+
21
+ const newData = foo.map(val => {
22
+ bar.someMethod(val);
23
+ isChanged = true;
24
+ });
25
+
26
+ if (isChanged) {
27
+ setFoo(newData);
28
+ }
29
+ }, [foo, bar]);
30
+
31
+ return (
32
+ <div>
33
+ {foo}, {bar}
34
+ </div>
35
+ );
36
+}
37
+
38
+```
39
+
40
+## Code
41
+
42
+```javascript
43
+import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
44
+
45
+function Component() {
46
+ const $ = _c(9);
47
+ let t0;
48
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
49
+ t0 = {};
50
+ $[0] = t0;
51
+ } else {
52
+ t0 = $[0];
53
+ }
54
+ const [foo, setFoo] = useState(t0);
55
+ let t1;
56
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
57
+ t1 = new Set();
58
+ $[1] = t1;
59
+ } else {
60
+ t1 = $[1];
61
+ }
62
+ const [bar] = useState(t1);
63
+ let t2;
64
+ let t3;
65
+ if ($[2] !== bar || $[3] !== foo) {
66
+ t2 = () => {
67
+ let isChanged = false;
68
+
69
+ const newData = foo.map((val) => {
70
+ bar.someMethod(val);
71
+ isChanged = true;
72
+ });
73
+
74
+ if (isChanged) {
75
+ setFoo(newData);
76
+ }
77
+ };
78
+
79
+ t3 = [foo, bar];
80
+ $[2] = bar;
81
+ $[3] = foo;
82
+ $[4] = t2;
83
+ $[5] = t3;
84
+ } else {
85
+ t2 = $[4];
86
+ t3 = $[5];
87
+ }
88
+ useEffect(t2, t3);
89
+ let t4;
90
+ if ($[6] !== bar || $[7] !== foo) {
91
+ t4 = (
92
+ <div>
93
+ {foo}, {bar}
94
+ </div>
95
+ );
96
+ $[6] = bar;
97
+ $[7] = foo;
98
+ $[8] = t4;
99
+ } else {
100
+ t4 = $[8];
101
+ }
102
+ return t4;
103
+}
104
+
105
+```
106
+
107
+## Logs
108
+
109
+```
110
+{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nState: [foo, bar]\n\nData Flow Tree:\n└── newData\n ├── foo (State)\n └── bar (State)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":23,"column":6,"index":663},"end":{"line":23,"column":12,"index":669},"filename":"function-expression-mutation-edge-case.ts","identifierName":"setFoo"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
111
+{"kind":"CompileSuccess","fnLoc":{"start":{"line":3,"column":0,"index":64},"end":{"line":32,"column":1,"index":762},"filename":"function-expression-mutation-edge-case.ts"},"fnName":"Component","memoSlots":9,"memoBlocks":4,"memoValues":5,"prunedMemoBlocks":0,"prunedMemoValues":0}
112
+```
113
+
114
+### Eval output
115
+(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/function-expression-mutation-edge-case.js
new
+32
@@ -0,0 +1,32 @@
1
+// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
2
+
3
+function Component() {
4
+ const [foo, setFoo] = useState({});
5
+ const [bar, setBar] = useState(new Set());
6
+
7
+ /*
8
+ * isChanged is considered context of the effect's function expression,
9
+ * if we don't bail out of effect mutation derivation tracking, isChanged
10
+ * will inherit the sources of the effect's function expression.
11
+ *
12
+ * This is innacurate and with the multiple passes ends up causing an infinite loop.
13
+ */
14
+ useEffect(() => {
15
+ let isChanged = false;
16
+
17
+ const newData = foo.map(val => {
18
+ bar.someMethod(val);
19
+ isChanged = true;
20
+ });
21
+
22
+ if (isChanged) {
23
+ setFoo(newData);
24
+ }
25
+ }, [foo, bar]);
26
+
27
+ return (
28
+ <div>
29
+ {foo}, {bar}
30
+ </div>
31
+ );
32
+}