Skip to content

Commit 0a576d7

Browse files
committed
[compiler] Fix false positive memo validation (alternative) (#34319)
Alternative to #34276 --- (Summary taken from @josephsavona 's #34276) Partial fix for #34262. Consider this example: ```js function useInputValue(input) { const object = React.useMemo(() => { const {value} = transform(input); return {value}; }, [input]); return object; } ``` React Compiler breaks this code into two reactive scopes: * One for `transform(input)` * One for `{value}` When we run ValidatePreserveExistingMemo, we see that the scope for `{value}` has the dependency `value`, whereas the original memoization had the dependency `input`, and throw an error that the dependencies didn't match. In other words, we're flagging the fact that memoized _better than the user_ as a problem. The more complete solution would be to validate that there is a subgraph of reactive scopes with a single input and output node, where the input node has the same dependencies as the original useMemo, and the output has the same outputs. That is true in this case, with the subgraph being the two consecutive scopes mentioned above. But that's complicated. As a shortcut, this PR checks for any dependencies that are defined after the start of the original useMemo. If we find one, we know that it's a case where we were able to memoize more precisely than the original, and we don't report an error on the dependency. We still check that the original _output_ value is able to be memoized, though. So if the scope of `object` were extended, eg with a call to `mutate(object)`, then we'd still correctly report an error that we couldn't preserve memoization. Co-authored-by: Joe Savona <[email protected]> DiffTrain build for [eda778b](eda778b)
1 parent 3e6538c commit 0a576d7

35 files changed

+117
-120
lines changed

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

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -49869,41 +49869,43 @@ class Visitor extends ReactiveFunctionVisitor {
4986949869
for (const instr of value.instructions) {
4987049870
this.visitInstruction(instr, state);
4987149871
}
49872-
const result = this.recordDepsInValue(value.value, state);
49873-
return result;
49872+
this.recordDepsInValue(value.value, state);
49873+
break;
4987449874
}
4987549875
case 'OptionalExpression': {
49876-
return this.recordDepsInValue(value.value, state);
49876+
this.recordDepsInValue(value.value, state);
49877+
break;
4987749878
}
4987849879
case 'ConditionalExpression': {
4987949880
this.recordDepsInValue(value.test, state);
4988049881
this.recordDepsInValue(value.consequent, state);
4988149882
this.recordDepsInValue(value.alternate, state);
49882-
return null;
49883+
break;
4988349884
}
4988449885
case 'LogicalExpression': {
4988549886
this.recordDepsInValue(value.left, state);
4988649887
this.recordDepsInValue(value.right, state);
49887-
return null;
49888+
break;
4988849889
}
4988949890
default: {
49890-
const dep = collectMaybeMemoDependencies(value, this.temporaries, false);
49891-
if (value.kind === 'StoreLocal' || value.kind === 'StoreContext') {
49892-
const storeTarget = value.lvalue.place;
49893-
(_a = state.manualMemoState) === null || _a === void 0 ? void 0 : _a.decls.add(storeTarget.identifier.declarationId);
49894-
if (((_b = storeTarget.identifier.name) === null || _b === void 0 ? void 0 : _b.kind) === 'named' && dep == null) {
49895-
const dep = {
49896-
root: {
49897-
kind: 'NamedLocal',
49898-
value: storeTarget,
49899-
},
49900-
path: [],
49901-
};
49902-
this.temporaries.set(storeTarget.identifier.id, dep);
49903-
return dep;
49891+
collectMaybeMemoDependencies(value, this.temporaries, false);
49892+
if (value.kind === 'StoreLocal' ||
49893+
value.kind === 'StoreContext' ||
49894+
value.kind === 'Destructure') {
49895+
for (const storeTarget of eachInstructionValueLValue(value)) {
49896+
(_a = state.manualMemoState) === null || _a === void 0 ? void 0 : _a.decls.add(storeTarget.identifier.declarationId);
49897+
if (((_b = storeTarget.identifier.name) === null || _b === void 0 ? void 0 : _b.kind) === 'named') {
49898+
this.temporaries.set(storeTarget.identifier.id, {
49899+
root: {
49900+
kind: 'NamedLocal',
49901+
value: storeTarget,
49902+
},
49903+
path: [],
49904+
});
49905+
}
4990449906
}
4990549907
}
49906-
return dep;
49908+
break;
4990749909
}
4990849910
}
4990949911
}
@@ -49919,20 +49921,15 @@ class Visitor extends ReactiveFunctionVisitor {
4991949921
if (lvalue !== null && isNamedLocal && state.manualMemoState != null) {
4992049922
state.manualMemoState.decls.add(lvalue.identifier.declarationId);
4992149923
}
49922-
const maybeDep = this.recordDepsInValue(value, state);
49923-
if (lvalId != null) {
49924-
if (maybeDep != null) {
49925-
temporaries.set(lvalId, maybeDep);
49926-
}
49927-
else if (isNamedLocal) {
49928-
temporaries.set(lvalId, {
49929-
root: {
49930-
kind: 'NamedLocal',
49931-
value: Object.assign({}, instr.lvalue),
49932-
},
49933-
path: [],
49934-
});
49935-
}
49924+
this.recordDepsInValue(value, state);
49925+
if (lvalue != null) {
49926+
temporaries.set(lvalue.identifier.id, {
49927+
root: {
49928+
kind: 'NamedLocal',
49929+
value: Object.assign({}, lvalue),
49930+
},
49931+
path: [],
49932+
});
4993649933
}
4993749934
}
4993849935
visitScope(scopeBlock, state) {

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
a9410fb487776339ec68e57a57a570be952ccad0
1+
eda778b8ae1698fec5fc84ca2530727df8b557b5
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
a9410fb487776339ec68e57a57a570be952ccad0
1+
eda778b8ae1698fec5fc84ca2530727df8b557b5

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1418,7 +1418,7 @@ __DEV__ &&
14181418
exports.useTransition = function () {
14191419
return resolveDispatcher().useTransition();
14201420
};
1421-
exports.version = "19.2.0-www-classic-a9410fb4-20250909";
1421+
exports.version = "19.2.0-www-classic-eda778b8-20250909";
14221422
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
14231423
"function" ===
14241424
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
@@ -1418,7 +1418,7 @@ __DEV__ &&
14181418
exports.useTransition = function () {
14191419
return resolveDispatcher().useTransition();
14201420
};
1421-
exports.version = "19.2.0-www-modern-a9410fb4-20250909";
1421+
exports.version = "19.2.0-www-modern-eda778b8-20250909";
14221422
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
14231423
"function" ===
14241424
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
@@ -600,4 +600,4 @@ exports.useSyncExternalStore = function (
600600
exports.useTransition = function () {
601601
return ReactSharedInternals.H.useTransition();
602602
};
603-
exports.version = "19.2.0-www-classic-a9410fb4-20250909";
603+
exports.version = "19.2.0-www-classic-eda778b8-20250909";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,4 +600,4 @@ exports.useSyncExternalStore = function (
600600
exports.useTransition = function () {
601601
return ReactSharedInternals.H.useTransition();
602602
};
603-
exports.version = "19.2.0-www-modern-a9410fb4-20250909";
603+
exports.version = "19.2.0-www-modern-eda778b8-20250909";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ exports.useSyncExternalStore = function (
604604
exports.useTransition = function () {
605605
return ReactSharedInternals.H.useTransition();
606606
};
607-
exports.version = "19.2.0-www-classic-a9410fb4-20250909";
607+
exports.version = "19.2.0-www-classic-eda778b8-20250909";
608608
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
609609
"function" ===
610610
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
@@ -604,7 +604,7 @@ exports.useSyncExternalStore = function (
604604
exports.useTransition = function () {
605605
return ReactSharedInternals.H.useTransition();
606606
};
607-
exports.version = "19.2.0-www-modern-a9410fb4-20250909";
607+
exports.version = "19.2.0-www-modern-eda778b8-20250909";
608608
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
609609
"function" ===
610610
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
@@ -19708,10 +19708,10 @@ __DEV__ &&
1970819708
(function () {
1970919709
var internals = {
1971019710
bundleType: 1,
19711-
version: "19.2.0-www-classic-a9410fb4-20250909",
19711+
version: "19.2.0-www-classic-eda778b8-20250909",
1971219712
rendererPackageName: "react-art",
1971319713
currentDispatcherRef: ReactSharedInternals,
19714-
reconcilerVersion: "19.2.0-www-classic-a9410fb4-20250909"
19714+
reconcilerVersion: "19.2.0-www-classic-eda778b8-20250909"
1971519715
};
1971619716
internals.overrideHookState = overrideHookState;
1971719717
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -19745,7 +19745,7 @@ __DEV__ &&
1974519745
exports.Shape = Shape;
1974619746
exports.Surface = Surface;
1974719747
exports.Text = Text;
19748-
exports.version = "19.2.0-www-classic-a9410fb4-20250909";
19748+
exports.version = "19.2.0-www-classic-eda778b8-20250909";
1974919749
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
1975019750
"function" ===
1975119751
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)