[compiler] Prevent overriding a derivationEntry on effect mutation and instead update typeOfValue and fix infinite loops (#34967)
Summary: With this we are now comparing a snapshot of the derivationCache with the new changes every time we are done recording the derivations happening in the HIR. We have to do this after recording everything since we still do some mutations on the cache when recording mutations. Test Plan: Test the following in playground: ``` // @validateNoDerivedComputationsInEffects_exp function Component({ value }) { const [checked, setChecked] = useState(''); useEffect(() => { setChecked(value === '' ? [] : value.split(',')); }, [value]); return ( <div>{checked}</div> ) } ``` This no longer causes an infinite loop. Added a test case in the next PR in the stack --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34967). * #35044 * #35020 * #34973 * #34972 * #34995 * __->__ #34967
Jorge Cabiedes committed
Nov 10, 2025 at 12:08 UTC
01fb3286321b6190b06b1cc86c7c1cd9e2d884d9
1 file changed
+59
-15
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts
+59
-15
@@ -47,6 +47,43 @@ type ValidationContext = {
47
class DerivationCache {
48
hasChanges: boolean = false;
49
cache: Map<IdentifierId, DerivationMetadata> = new Map();
50
+ private previousCache: Map<IdentifierId, DerivationMetadata> | null = null;
51
+
52
+ takeSnapshot(): void {
53
+ this.previousCache = new Map();
54
+ for (const [key, value] of this.cache.entries()) {
55
+ this.previousCache.set(key, {
56
+ place: value.place,
57
+ sourcesIds: new Set(value.sourcesIds),
58
+ typeOfValue: value.typeOfValue,
59
+ });
60
+ }
61
+ }
62
+
63
+ checkForChanges(): void {
64
+ if (this.previousCache === null) {
65
+ this.hasChanges = true;
66
+ return;
67
+ }
68
+
69
+ for (const [key, value] of this.cache.entries()) {
70
+ const previousValue = this.previousCache.get(key);
71
+ if (
72
+ previousValue === undefined ||
73
+ !this.isDerivationEqual(previousValue, value)
74
+ ) {
75
+ this.hasChanges = true;
76
+ return;
77
+ }
78
+ }
79
+
80
+ if (this.cache.size !== this.previousCache.size) {
81
+ this.hasChanges = true;
82
+ return;
83
+ }
84
+
85
+ this.hasChanges = false;
86
+ }
87
88
snapshot(): boolean {
89
const hasChanges = this.hasChanges;
@@ -92,14 +129,7 @@ class DerivationCache {
129
newValue.sourcesIds.add(derivedVar.identifier.id);
130
}
131
95
- const existingValue = this.cache.get(derivedVar.identifier.id);
96
- if (
97
- existingValue === undefined ||
98
- !this.isDerivationEqual(existingValue, newValue)
99
- ) {
100
- this.cache.set(derivedVar.identifier.id, newValue);
101
- this.hasChanges = true;
102
- }
132
+ this.cache.set(derivedVar.identifier.id, newValue);
133
}
134
135
private isDerivationEqual(
@@ -175,7 +205,6 @@ export function validateNoDerivedComputationsInEffects_exp(
205
sourcesIds: new Set([param.identifier.id]),
206
typeOfValue: 'fromProps',
207
});
178
- context.derivationCache.hasChanges = true;
208
}
209
}
210
} else if (fn.fnType === 'Component') {
@@ -186,12 +215,13 @@ export function validateNoDerivedComputationsInEffects_exp(
215
sourcesIds: new Set([props.identifier.id]),
216
typeOfValue: 'fromProps',
217
});
189
- context.derivationCache.hasChanges = true;
218
}
219
}
220
221
let isFirstPass = true;
222
do {
223
+ context.derivationCache.takeSnapshot();
224
+
225
for (const block of fn.body.blocks.values()) {
226
recordPhiDerivations(block, context);
227
for (const instr of block.instructions) {
@@ -199,6 +229,7 @@ export function validateNoDerivedComputationsInEffects_exp(
229
}
230
}
231
232
+ context.derivationCache.checkForChanges();
233
isFirstPass = false;
234
} while (context.derivationCache.snapshot());
235
@@ -331,11 +362,24 @@ function recordInstructionDerivations(
362
case Effect.ConditionallyMutateIterator:
363
case Effect.Mutate: {
364
if (isMutable(instr, operand)) {
334
- context.derivationCache.addDerivationEntry(
335
- operand,
336
- sources,
337
- typeOfValue,
338
- );
365
+ if (context.derivationCache.cache.has(operand.identifier.id)) {
366
+ const operandMetadata = context.derivationCache.cache.get(
367
+ operand.identifier.id,
368
+ );
369
+
370
+ if (operandMetadata !== undefined) {
371
+ operandMetadata.typeOfValue = joinValue(
372
+ typeOfValue,
373
+ operandMetadata.typeOfValue,
374
+ );
375
+ }
376
+ } else {
377
+ context.derivationCache.addDerivationEntry(
378
+ operand,
379
+ sources,
380
+ typeOfValue,
381
+ );
382
+ }
383
}
384
break;
385
}