Skip to content

Commit af0a333

Browse files
committed
[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 DiffTrain build for [01fb328](01fb328)
1 parent aad8576 commit af0a333

35 files changed

+128
-95
lines changed

compiled/eslint-plugin-react-hooks/index.js

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52230,6 +52230,36 @@ class DerivationCache {
5223052230
constructor() {
5223152231
this.hasChanges = false;
5223252232
this.cache = new Map();
52233+
this.previousCache = null;
52234+
}
52235+
takeSnapshot() {
52236+
this.previousCache = new Map();
52237+
for (const [key, value] of this.cache.entries()) {
52238+
this.previousCache.set(key, {
52239+
place: value.place,
52240+
sourcesIds: new Set(value.sourcesIds),
52241+
typeOfValue: value.typeOfValue,
52242+
});
52243+
}
52244+
}
52245+
checkForChanges() {
52246+
if (this.previousCache === null) {
52247+
this.hasChanges = true;
52248+
return;
52249+
}
52250+
for (const [key, value] of this.cache.entries()) {
52251+
const previousValue = this.previousCache.get(key);
52252+
if (previousValue === undefined ||
52253+
!this.isDerivationEqual(previousValue, value)) {
52254+
this.hasChanges = true;
52255+
return;
52256+
}
52257+
}
52258+
if (this.cache.size !== this.previousCache.size) {
52259+
this.hasChanges = true;
52260+
return;
52261+
}
52262+
this.hasChanges = false;
5223352263
}
5223452264
snapshot() {
5223552265
const hasChanges = this.hasChanges;
@@ -52261,12 +52291,7 @@ class DerivationCache {
5226152291
if (newValue.sourcesIds.size === 0) {
5226252292
newValue.sourcesIds.add(derivedVar.identifier.id);
5226352293
}
52264-
const existingValue = this.cache.get(derivedVar.identifier.id);
52265-
if (existingValue === undefined ||
52266-
!this.isDerivationEqual(existingValue, newValue)) {
52267-
this.cache.set(derivedVar.identifier.id, newValue);
52268-
this.hasChanges = true;
52269-
}
52294+
this.cache.set(derivedVar.identifier.id, newValue);
5227052295
}
5227152296
isDerivationEqual(a, b) {
5227252297
if (a.typeOfValue !== b.typeOfValue) {
@@ -52306,7 +52331,6 @@ function validateNoDerivedComputationsInEffects_exp(fn) {
5230652331
sourcesIds: new Set([param.identifier.id]),
5230752332
typeOfValue: 'fromProps',
5230852333
});
52309-
context.derivationCache.hasChanges = true;
5231052334
}
5231152335
}
5231252336
}
@@ -52318,17 +52342,18 @@ function validateNoDerivedComputationsInEffects_exp(fn) {
5231852342
sourcesIds: new Set([props.identifier.id]),
5231952343
typeOfValue: 'fromProps',
5232052344
});
52321-
context.derivationCache.hasChanges = true;
5232252345
}
5232352346
}
5232452347
let isFirstPass = true;
5232552348
do {
52349+
context.derivationCache.takeSnapshot();
5232652350
for (const block of fn.body.blocks.values()) {
5232752351
recordPhiDerivations(block, context);
5232852352
for (const instr of block.instructions) {
5232952353
recordInstructionDerivations(instr, context, isFirstPass);
5233052354
}
5233152355
}
52356+
context.derivationCache.checkForChanges();
5233252357
isFirstPass = false;
5233352358
} while (context.derivationCache.snapshot());
5233452359
for (const effect of effects) {
@@ -52429,7 +52454,15 @@ function recordInstructionDerivations(instr, context, isFirstPass) {
5242952454
case Effect.ConditionallyMutateIterator:
5243052455
case Effect.Mutate: {
5243152456
if (isMutable(instr, operand)) {
52432-
context.derivationCache.addDerivationEntry(operand, sources, typeOfValue);
52457+
if (context.derivationCache.cache.has(operand.identifier.id)) {
52458+
const operandMetadata = context.derivationCache.cache.get(operand.identifier.id);
52459+
if (operandMetadata !== undefined) {
52460+
operandMetadata.typeOfValue = joinValue(typeOfValue, operandMetadata.typeOfValue);
52461+
}
52462+
}
52463+
else {
52464+
context.derivationCache.addDerivationEntry(operand, sources, typeOfValue);
52465+
}
5243352466
}
5243452467
break;
5243552468
}

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
52684925368a41a0c9fbfca9016cdcbb72fc9d1e
1+
01fb3286321b6190b06b1cc86c7c1cd9e2d884d9
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
52684925368a41a0c9fbfca9016cdcbb72fc9d1e
1+
01fb3286321b6190b06b1cc86c7c1cd9e2d884d9

compiled/facebook-www/React-dev.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1499,7 +1499,7 @@ __DEV__ &&
14991499
exports.useTransition = function () {
15001500
return resolveDispatcher().useTransition();
15011501
};
1502-
exports.version = "19.3.0-www-classic-52684925-20251110";
1502+
exports.version = "19.3.0-www-classic-01fb3286-20251110";
15031503
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
15041504
"function" ===
15051505
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-dev.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1499,7 +1499,7 @@ __DEV__ &&
14991499
exports.useTransition = function () {
15001500
return resolveDispatcher().useTransition();
15011501
};
1502-
exports.version = "19.3.0-www-modern-52684925-20251110";
1502+
exports.version = "19.3.0-www-modern-01fb3286-20251110";
15031503
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
15041504
"function" ===
15051505
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-prod.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,4 +606,4 @@ exports.useSyncExternalStore = function (
606606
exports.useTransition = function () {
607607
return ReactSharedInternals.H.useTransition();
608608
};
609-
exports.version = "19.3.0-www-classic-52684925-20251110";
609+
exports.version = "19.3.0-www-classic-01fb3286-20251110";

compiled/facebook-www/React-prod.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,4 +606,4 @@ exports.useSyncExternalStore = function (
606606
exports.useTransition = function () {
607607
return ReactSharedInternals.H.useTransition();
608608
};
609-
exports.version = "19.3.0-www-modern-52684925-20251110";
609+
exports.version = "19.3.0-www-modern-01fb3286-20251110";

compiled/facebook-www/React-profiling.classic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ exports.useSyncExternalStore = function (
610610
exports.useTransition = function () {
611611
return ReactSharedInternals.H.useTransition();
612612
};
613-
exports.version = "19.3.0-www-classic-52684925-20251110";
613+
exports.version = "19.3.0-www-classic-01fb3286-20251110";
614614
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
615615
"function" ===
616616
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/React-profiling.modern.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ exports.useSyncExternalStore = function (
610610
exports.useTransition = function () {
611611
return ReactSharedInternals.H.useTransition();
612612
};
613-
exports.version = "19.3.0-www-modern-52684925-20251110";
613+
exports.version = "19.3.0-www-modern-01fb3286-20251110";
614614
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
615615
"function" ===
616616
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

compiled/facebook-www/ReactART-dev.classic.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20466,10 +20466,10 @@ __DEV__ &&
2046620466
(function () {
2046720467
var internals = {
2046820468
bundleType: 1,
20469-
version: "19.3.0-www-classic-52684925-20251110",
20469+
version: "19.3.0-www-classic-01fb3286-20251110",
2047020470
rendererPackageName: "react-art",
2047120471
currentDispatcherRef: ReactSharedInternals,
20472-
reconcilerVersion: "19.3.0-www-classic-52684925-20251110"
20472+
reconcilerVersion: "19.3.0-www-classic-01fb3286-20251110"
2047320473
};
2047420474
internals.overrideHookState = overrideHookState;
2047520475
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -20504,7 +20504,7 @@ __DEV__ &&
2050420504
exports.Shape = Shape;
2050520505
exports.Surface = Surface;
2050620506
exports.Text = Text;
20507-
exports.version = "19.3.0-www-classic-52684925-20251110";
20507+
exports.version = "19.3.0-www-classic-01fb3286-20251110";
2050820508
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
2050920509
"function" ===
2051020510
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)