Skip to content

Commit cf7bb43

Browse files
committed
[compiler] Add new ValidateNoVoidUseMemo pass (#33990)
Adds a new validation pass to validate against `useMemo`s that don't return anything. This usually indicates some kind of "useEffect"-like code that has side effects that need to be memoized to prevent overfiring, and is an anti-pattern. A follow up validation could also look at the return value of `useMemo`s to see if they are being used. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33990). * #34022 * #34002 * #34001 * __->__ #33990 * #33989 DiffTrain build for [c60eebf](c60eebf)
1 parent b723ffd commit cf7bb43

35 files changed

+193
-88
lines changed

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

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30734,6 +30734,7 @@ const EnvironmentConfigSchema = zod.z.object({
3073430734
hookPattern: zod.z.string().nullable().default(null),
3073530735
enableTreatRefLikeIdentifiersAsRefs: zod.z.boolean().default(false),
3073630736
lowerContextAccess: ExternalFunctionSchema.nullable().default(null),
30737+
validateNoVoidUseMemo: zod.z.boolean().default(false),
3073730738
});
3073830739
class Environment {
3073930740
constructor(scope, fnType, compilerMode, config, contextIdentifiers, parentFunction, logger, filename, code, programContext) {
@@ -43754,7 +43755,7 @@ function collectMaybeMemoDependencies(value, maybeDeps, optional) {
4375443755
}
4375543756
return null;
4375643757
}
43757-
function collectTemporaries(instr, env, sidemap) {
43758+
function collectTemporaries$1(instr, env, sidemap) {
4375843759
const { value, lvalue } = instr;
4375943760
switch (value.kind) {
4376043761
case 'FunctionExpression': {
@@ -43945,7 +43946,7 @@ function dropManualMemoization(func) {
4394543946
}
4394643947
}
4394743948
else {
43948-
collectTemporaries(instr, func.env, sidemap);
43949+
collectTemporaries$1(instr, func.env, sidemap);
4394943950
}
4395043951
}
4395143952
}
@@ -47617,6 +47618,107 @@ function visit(identifiers, place, kind) {
4761747618
identifiers.set(place.identifier.id, { place, kind });
4761847619
}
4761947620

47621+
function validateNoVoidUseMemo(fn) {
47622+
const errors = new CompilerError();
47623+
const sidemap = {
47624+
useMemoHooks: new Map(),
47625+
funcExprs: new Map(),
47626+
react: new Set(),
47627+
};
47628+
for (const [, block] of fn.body.blocks) {
47629+
for (const instr of block.instructions) {
47630+
collectTemporaries(instr, fn.env, sidemap);
47631+
}
47632+
}
47633+
for (const [, block] of fn.body.blocks) {
47634+
for (const instr of block.instructions) {
47635+
if (instr.value.kind === 'CallExpression') {
47636+
const callee = instr.value.callee.identifier;
47637+
const useMemoHook = sidemap.useMemoHooks.get(callee.id);
47638+
if (useMemoHook !== undefined && instr.value.args.length > 0) {
47639+
const firstArg = instr.value.args[0];
47640+
if (firstArg.kind !== 'Identifier') {
47641+
continue;
47642+
}
47643+
let funcToCheck = sidemap.funcExprs.get(firstArg.identifier.id);
47644+
if (!funcToCheck) {
47645+
for (const [, searchBlock] of fn.body.blocks) {
47646+
for (const searchInstr of searchBlock.instructions) {
47647+
if (searchInstr.lvalue &&
47648+
searchInstr.lvalue.identifier.id === firstArg.identifier.id &&
47649+
searchInstr.value.kind === 'FunctionExpression') {
47650+
funcToCheck = searchInstr.value;
47651+
break;
47652+
}
47653+
}
47654+
if (funcToCheck)
47655+
break;
47656+
}
47657+
}
47658+
if (funcToCheck) {
47659+
const hasReturn = checkFunctionHasNonVoidReturn(funcToCheck.loweredFunc.func);
47660+
if (!hasReturn) {
47661+
errors.push({
47662+
severity: ErrorSeverity.InvalidReact,
47663+
reason: `React Compiler has skipped optimizing this component because ${useMemoHook.name} doesn't return a value. ${useMemoHook.name} should only be used for memoizing values, not running arbitrary side effects.`,
47664+
loc: useMemoHook.loc,
47665+
suggestions: null,
47666+
description: null,
47667+
});
47668+
}
47669+
}
47670+
}
47671+
}
47672+
}
47673+
}
47674+
return errors.asResult();
47675+
}
47676+
function checkFunctionHasNonVoidReturn(func) {
47677+
for (const [, block] of func.body.blocks) {
47678+
if (block.terminal.kind === 'return') {
47679+
if (block.terminal.returnVariant === 'Explicit' ||
47680+
block.terminal.returnVariant === 'Implicit') {
47681+
return true;
47682+
}
47683+
}
47684+
}
47685+
return false;
47686+
}
47687+
function collectTemporaries(instr, env, sidemap) {
47688+
const { value, lvalue } = instr;
47689+
switch (value.kind) {
47690+
case 'FunctionExpression': {
47691+
sidemap.funcExprs.set(lvalue.identifier.id, value);
47692+
break;
47693+
}
47694+
case 'LoadGlobal': {
47695+
const global = env.getGlobalDeclaration(value.binding, value.loc);
47696+
const hookKind = global !== null ? getHookKindForType(env, global) : null;
47697+
if (hookKind === 'useMemo') {
47698+
sidemap.useMemoHooks.set(lvalue.identifier.id, {
47699+
name: value.binding.name,
47700+
loc: instr.loc,
47701+
});
47702+
}
47703+
else if (value.binding.name === 'React') {
47704+
sidemap.react.add(lvalue.identifier.id);
47705+
}
47706+
break;
47707+
}
47708+
case 'PropertyLoad': {
47709+
if (sidemap.react.has(value.object.identifier.id)) {
47710+
if (value.property === 'useMemo') {
47711+
sidemap.useMemoHooks.set(lvalue.identifier.id, {
47712+
name: value.property,
47713+
loc: instr.loc,
47714+
});
47715+
}
47716+
}
47717+
break;
47718+
}
47719+
}
47720+
}
47721+
4762047722
function computeUnconditionalBlocks(fn) {
4762147723
const unconditionalBlocks = new Set();
4762247724
const dominators = computePostDominatorTree(fn, {
@@ -50709,6 +50811,9 @@ function runWithEnvironment(func, env) {
5070950811
log({ kind: 'hir', name: 'PruneMaybeThrows', value: hir });
5071050812
validateContextVariableLValues(hir);
5071150813
validateUseMemo(hir).unwrap();
50814+
if (env.config.validateNoVoidUseMemo) {
50815+
validateNoVoidUseMemo(hir).unwrap();
50816+
}
5071250817
if (env.isInferredMemoEnabled &&
5071350818
!env.config.enablePreserveExistingManualUseMemo &&
5071450819
!env.config.disableMemoizationForDebugging &&

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
5dd622eabe38e01781b3699d0d81c4a16e302f09
1+
c60eebffea94a67f35c6ebbf7019e5b2145d4284
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
5dd622eabe38e01781b3699d0d81c4a16e302f09
1+
c60eebffea94a67f35c6ebbf7019e5b2145d4284

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1434,7 +1434,7 @@ __DEV__ &&
14341434
exports.useTransition = function () {
14351435
return resolveDispatcher().useTransition();
14361436
};
1437-
exports.version = "19.2.0-www-classic-5dd622ea-20250728";
1437+
exports.version = "19.2.0-www-classic-c60eebff-20250728";
14381438
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
14391439
"function" ===
14401440
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
@@ -1434,7 +1434,7 @@ __DEV__ &&
14341434
exports.useTransition = function () {
14351435
return resolveDispatcher().useTransition();
14361436
};
1437-
exports.version = "19.2.0-www-modern-5dd622ea-20250728";
1437+
exports.version = "19.2.0-www-modern-c60eebff-20250728";
14381438
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
14391439
"function" ===
14401440
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
@@ -610,4 +610,4 @@ exports.useSyncExternalStore = function (
610610
exports.useTransition = function () {
611611
return ReactSharedInternals.H.useTransition();
612612
};
613-
exports.version = "19.2.0-www-classic-5dd622ea-20250728";
613+
exports.version = "19.2.0-www-classic-c60eebff-20250728";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,4 +610,4 @@ exports.useSyncExternalStore = function (
610610
exports.useTransition = function () {
611611
return ReactSharedInternals.H.useTransition();
612612
};
613-
exports.version = "19.2.0-www-modern-5dd622ea-20250728";
613+
exports.version = "19.2.0-www-modern-c60eebff-20250728";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ exports.useSyncExternalStore = function (
614614
exports.useTransition = function () {
615615
return ReactSharedInternals.H.useTransition();
616616
};
617-
exports.version = "19.2.0-www-classic-5dd622ea-20250728";
617+
exports.version = "19.2.0-www-classic-c60eebff-20250728";
618618
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
619619
"function" ===
620620
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
@@ -614,7 +614,7 @@ exports.useSyncExternalStore = function (
614614
exports.useTransition = function () {
615615
return ReactSharedInternals.H.useTransition();
616616
};
617-
exports.version = "19.2.0-www-modern-5dd622ea-20250728";
617+
exports.version = "19.2.0-www-modern-c60eebff-20250728";
618618
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
619619
"function" ===
620620
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
@@ -19305,10 +19305,10 @@ __DEV__ &&
1930519305
(function () {
1930619306
var internals = {
1930719307
bundleType: 1,
19308-
version: "19.2.0-www-classic-5dd622ea-20250728",
19308+
version: "19.2.0-www-classic-c60eebff-20250728",
1930919309
rendererPackageName: "react-art",
1931019310
currentDispatcherRef: ReactSharedInternals,
19311-
reconcilerVersion: "19.2.0-www-classic-5dd622ea-20250728"
19311+
reconcilerVersion: "19.2.0-www-classic-c60eebff-20250728"
1931219312
};
1931319313
internals.overrideHookState = overrideHookState;
1931419314
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -19342,7 +19342,7 @@ __DEV__ &&
1934219342
exports.Shape = Shape;
1934319343
exports.Surface = Surface;
1934419344
exports.Text = Text;
19345-
exports.version = "19.2.0-www-classic-5dd622ea-20250728";
19345+
exports.version = "19.2.0-www-classic-c60eebff-20250728";
1934619346
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
1934719347
"function" ===
1934819348
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)