Skip to content

Commit 70171ce

Browse files
committed
[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 DiffTrain build for [7ee974d](7ee974d)
1 parent 99090b7 commit 70171ce

35 files changed

+114
-86
lines changed

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52496,6 +52496,7 @@ function validateEffect$1(effectFunction, effectDeps, errors) {
5249652496
}
5249752497
}
5249852498

52499+
const MAX_FIXPOINT_ITERATIONS = 100;
5249952500
class DerivationCache {
5250052501
constructor() {
5250152502
this.hasChanges = false;
@@ -52615,6 +52616,7 @@ function validateNoDerivedComputationsInEffects_exp(fn) {
5261552616
});
5261652617
}
5261752618
}
52619+
let iterationCount = 0;
5261852620
do {
5261952621
context.derivationCache.takeSnapshot();
5262052622
for (const block of fn.body.blocks.values()) {
@@ -52624,6 +52626,18 @@ function validateNoDerivedComputationsInEffects_exp(fn) {
5262452626
}
5262552627
}
5262652628
context.derivationCache.checkForChanges();
52629+
iterationCount++;
52630+
CompilerError.invariant(iterationCount < MAX_FIXPOINT_ITERATIONS, {
52631+
reason: '[ValidateNoDerivedComputationsInEffects] Fixpoint iteration failed to converge.',
52632+
description: `Fixpoint iteration exceeded ${MAX_FIXPOINT_ITERATIONS} iterations while tracking derivations. This suggests a cyclic dependency in the derivation cache.`,
52633+
details: [
52634+
{
52635+
kind: 'error',
52636+
loc: fn.loc,
52637+
message: `Exceeded ${MAX_FIXPOINT_ITERATIONS} iterations in ValidateNoDerivedComputationsInEffects`,
52638+
},
52639+
],
52640+
});
5262752641
} while (context.derivationCache.snapshot());
5262852642
for (const [, effect] of effectsCache) {
5262952643
validateEffect(effect.effect, effect.dependencies, context);
@@ -52747,6 +52761,9 @@ function recordInstructionDerivations(instr, context, isFirstPass) {
5274752761
for (const lvalue of eachInstructionLValue(instr)) {
5274852762
context.derivationCache.addDerivationEntry(lvalue, sources, typeOfValue, isSource);
5274952763
}
52764+
if (value.kind === 'FunctionExpression') {
52765+
return;
52766+
}
5275052767
for (const operand of eachInstructionOperand(instr)) {
5275152768
switch (operand.effect) {
5275252769
case Effect.Capture:
@@ -52808,6 +52825,17 @@ function buildTreeNode(sourceId, context, visited = new Set()) {
5280852825
const children = [];
5280952826
const namedSiblings = new Set();
5281052827
for (const childId of sourceMetadata.sourcesIds) {
52828+
CompilerError.invariant(childId !== sourceId, {
52829+
reason: 'Unexpected self-reference: a value should not have itself as a source',
52830+
description: null,
52831+
details: [
52832+
{
52833+
kind: 'error',
52834+
loc: sourceMetadata.place.loc,
52835+
message: null,
52836+
},
52837+
],
52838+
});
5281152839
const childNodes = buildTreeNode(childId, context, new Set([
5281252840
...visited,
5281352841
...(isNamedIdentifier(sourceMetadata.place)

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8ac5f4eb3601f7381462f8b74ecf24d47259cc20
1+
7ee974de927e9bbe10a44441ab49bafd9f5467a2
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8ac5f4eb3601f7381462f8b74ecf24d47259cc20
1+
7ee974de927e9bbe10a44441ab49bafd9f5467a2

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-8ac5f4eb-20251119";
1502+
exports.version = "19.3.0-www-classic-7ee974de-20251120";
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-8ac5f4eb-20251119";
1502+
exports.version = "19.3.0-www-modern-7ee974de-20251120";
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-8ac5f4eb-20251119";
609+
exports.version = "19.3.0-www-classic-7ee974de-20251120";

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-8ac5f4eb-20251119";
609+
exports.version = "19.3.0-www-modern-7ee974de-20251120";

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-8ac5f4eb-20251119";
613+
exports.version = "19.3.0-www-classic-7ee974de-20251120";
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-8ac5f4eb-20251119";
613+
exports.version = "19.3.0-www-modern-7ee974de-20251120";
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
@@ -20468,10 +20468,10 @@ __DEV__ &&
2046820468
(function () {
2046920469
var internals = {
2047020470
bundleType: 1,
20471-
version: "19.3.0-www-classic-8ac5f4eb-20251119",
20471+
version: "19.3.0-www-classic-7ee974de-20251120",
2047220472
rendererPackageName: "react-art",
2047320473
currentDispatcherRef: ReactSharedInternals,
20474-
reconcilerVersion: "19.3.0-www-classic-8ac5f4eb-20251119"
20474+
reconcilerVersion: "19.3.0-www-classic-7ee974de-20251120"
2047520475
};
2047620476
internals.overrideHookState = overrideHookState;
2047720477
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -20506,7 +20506,7 @@ __DEV__ &&
2050620506
exports.Shape = Shape;
2050720507
exports.Surface = Surface;
2050820508
exports.Text = Text;
20509-
exports.version = "19.3.0-www-classic-8ac5f4eb-20251119";
20509+
exports.version = "19.3.0-www-classic-7ee974de-20251120";
2051020510
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
2051120511
"function" ===
2051220512
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)