Skip to content

Commit 0958561

Browse files
committed
[compiler] Cleanup and enable validateNoVoidUseMemo (#34882)
This is a great validation, so let's enable by default. Changes: * Move the validation logic into ValidateUseMemo alongside the new check that the useMemo result is used * Update the lint description * Make the void memo errors lint-only, they don't require us to skip compilation (as evidenced by the fact that we've had this validation off) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34882). * #34855 * __->__ #34882 DiffTrain build for [1324e1b](1324e1b)
1 parent d437a65 commit 0958561

35 files changed

+118
-123
lines changed

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

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -18309,7 +18309,7 @@ function getRuleForCategoryImpl(category) {
1830918309
category,
1831018310
severity: ErrorSeverity.Error,
1831118311
name: 'void-use-memo',
18312-
description: 'Validates that useMemos always return a value. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.',
18312+
description: 'Validates that useMemos always return a value and that the result of the useMemo is used by the component/hook. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.',
1831318313
preset: LintRulePreset.RecommendedLatest,
1831418314
};
1831518315
}
@@ -32187,7 +32187,7 @@ const EnvironmentConfigSchema = v4.z.object({
3218732187
enableTreatRefLikeIdentifiersAsRefs: v4.z.boolean().default(true),
3218832188
enableTreatSetIdentifiersAsStateSetters: v4.z.boolean().default(false),
3218932189
lowerContextAccess: ExternalFunctionSchema.nullable().default(null),
32190-
validateNoVoidUseMemo: v4.z.boolean().default(false),
32190+
validateNoVoidUseMemo: v4.z.boolean().default(true),
3219132191
validateNoDynamicallyCreatedComponentsOrHooks: v4.z.boolean().default(false),
3219232192
enableAllowSetStateFromRefsInEffects: v4.z.boolean().default(true),
3219332193
});
@@ -44571,7 +44571,6 @@ function extractManualMemoizationArgs(instr, kind, sidemap, errors) {
4457144571
};
4457244572
}
4457344573
function dropManualMemoization(func) {
44574-
var _a;
4457544574
const errors = new CompilerError();
4457644575
const isValidationEnabled = func.env.config.validatePreserveExistingMemoizationGuarantees ||
4457744576
func.env.config.validateNoSetStateInRender ||
@@ -44601,26 +44600,6 @@ function dropManualMemoization(func) {
4460144600
if (fnPlace == null) {
4460244601
continue;
4460344602
}
44604-
if (func.env.config.validateNoVoidUseMemo &&
44605-
manualMemo.kind === 'useMemo') {
44606-
const funcToCheck = (_a = sidemap.functions.get(fnPlace.identifier.id)) === null || _a === void 0 ? void 0 : _a.value;
44607-
if (funcToCheck !== undefined && funcToCheck.loweredFunc.func) {
44608-
if (!hasNonVoidReturn(funcToCheck.loweredFunc.func)) {
44609-
errors.pushDiagnostic(CompilerDiagnostic.create({
44610-
category: ErrorCategory.VoidUseMemo,
44611-
reason: 'useMemo() callbacks must return a value',
44612-
description: `This ${manualMemo.loadInstr.value.kind === 'PropertyLoad'
44613-
? 'React.useMemo()'
44614-
: 'useMemo()'} callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`,
44615-
suggestions: null,
44616-
}).withDetails({
44617-
kind: 'error',
44618-
loc: instr.value.loc,
44619-
message: 'useMemo() callbacks must return a value',
44620-
}));
44621-
}
44622-
}
44623-
}
4462444603
instr.value = getManualMemoizationReplacement(fnPlace, instr.value.loc, manualMemo.kind);
4462544604
if (isValidationEnabled) {
4462644605
if (!sidemap.functions.has(fnPlace.identifier.id)) {
@@ -44732,17 +44711,6 @@ function findOptionalPlaces(fn) {
4473244711
}
4473344712
return optionals;
4473444713
}
44735-
function hasNonVoidReturn(func) {
44736-
for (const [, block] of func.body.blocks) {
44737-
if (block.terminal.kind === 'return') {
44738-
if (block.terminal.returnVariant === 'Explicit' ||
44739-
block.terminal.returnVariant === 'Implicit') {
44740-
return true;
44741-
}
44742-
}
44743-
}
44744-
return false;
44745-
}
4474644714

4474744715
class StableSidemap {
4474844716
constructor(env) {
@@ -50225,6 +50193,7 @@ function isUnmemoized(operand, scopes) {
5022550193

5022650194
function validateUseMemo(fn) {
5022750195
const errors = new CompilerError();
50196+
const voidMemoErrors = new CompilerError();
5022850197
const useMemos = new Set();
5022950198
const react = new Set();
5023050199
const functions = new Map();
@@ -50303,7 +50272,21 @@ function validateUseMemo(fn) {
5030350272
}
5030450273
validateNoContextVariableAssignment(body.loweredFunc.func, errors);
5030550274
if (fn.env.config.validateNoVoidUseMemo) {
50306-
unusedUseMemos.set(lvalue.identifier.id, callee.loc);
50275+
if (!hasNonVoidReturn(body.loweredFunc.func)) {
50276+
voidMemoErrors.pushDiagnostic(CompilerDiagnostic.create({
50277+
category: ErrorCategory.VoidUseMemo,
50278+
reason: 'useMemo() callbacks must return a value',
50279+
description: `This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`,
50280+
suggestions: null,
50281+
}).withDetails({
50282+
kind: 'error',
50283+
loc: body.loc,
50284+
message: 'useMemo() callbacks must return a value',
50285+
}));
50286+
}
50287+
else {
50288+
unusedUseMemos.set(lvalue.identifier.id, callee.loc);
50289+
}
5030750290
}
5030850291
break;
5030950292
}
@@ -50317,9 +50300,9 @@ function validateUseMemo(fn) {
5031750300
}
5031850301
if (unusedUseMemos.size !== 0) {
5031950302
for (const loc of unusedUseMemos.values()) {
50320-
errors.pushDiagnostic(CompilerDiagnostic.create({
50303+
voidMemoErrors.pushDiagnostic(CompilerDiagnostic.create({
5032150304
category: ErrorCategory.VoidUseMemo,
50322-
reason: 'Unused useMemo()',
50305+
reason: 'useMemo() result is unused',
5032350306
description: `This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects`,
5032450307
suggestions: null,
5032550308
}).withDetails({
@@ -50329,6 +50312,7 @@ function validateUseMemo(fn) {
5032950312
}));
5033050313
}
5033150314
}
50315+
fn.env.logErrors(voidMemoErrors.asResult());
5033250316
return errors.asResult();
5033350317
}
5033450318
function validateNoContextVariableAssignment(fn, errors) {
@@ -50353,6 +50337,17 @@ function validateNoContextVariableAssignment(fn, errors) {
5035350337
}
5035450338
}
5035550339
}
50340+
function hasNonVoidReturn(func) {
50341+
for (const [, block] of func.body.blocks) {
50342+
if (block.terminal.kind === 'return') {
50343+
if (block.terminal.returnVariant === 'Explicit' ||
50344+
block.terminal.returnVariant === 'Implicit') {
50345+
return true;
50346+
}
50347+
}
50348+
}
50349+
return false;
50350+
}
5035650351

5035750352
function validateLocalsNotReassignedAfterRender(fn) {
5035850353
const contextVariables = new Set();

compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
7f5ea1bf67a1a920919ebe2ae6657bdebe505aa0
1+
1324e1bb1f867e8b2108ca52a1d4e2d4ef56d2d9
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
7f5ea1bf67a1a920919ebe2ae6657bdebe505aa0
1+
1324e1bb1f867e8b2108ca52a1d4e2d4ef56d2d9

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1478,7 +1478,7 @@ __DEV__ &&
14781478
exports.useTransition = function () {
14791479
return resolveDispatcher().useTransition();
14801480
};
1481-
exports.version = "19.3.0-www-classic-7f5ea1bf-20251016";
1481+
exports.version = "19.3.0-www-classic-1324e1bb-20251016";
14821482
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
14831483
"function" ===
14841484
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
@@ -1478,7 +1478,7 @@ __DEV__ &&
14781478
exports.useTransition = function () {
14791479
return resolveDispatcher().useTransition();
14801480
};
1481-
exports.version = "19.3.0-www-modern-7f5ea1bf-20251016";
1481+
exports.version = "19.3.0-www-modern-1324e1bb-20251016";
14821482
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
14831483
"function" ===
14841484
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-7f5ea1bf-20251016";
609+
exports.version = "19.3.0-www-classic-1324e1bb-20251016";

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-7f5ea1bf-20251016";
609+
exports.version = "19.3.0-www-modern-1324e1bb-20251016";

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-7f5ea1bf-20251016";
613+
exports.version = "19.3.0-www-classic-1324e1bb-20251016";
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-7f5ea1bf-20251016";
613+
exports.version = "19.3.0-www-modern-1324e1bb-20251016";
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
@@ -20371,10 +20371,10 @@ __DEV__ &&
2037120371
(function () {
2037220372
var internals = {
2037320373
bundleType: 1,
20374-
version: "19.3.0-www-classic-7f5ea1bf-20251016",
20374+
version: "19.3.0-www-classic-1324e1bb-20251016",
2037520375
rendererPackageName: "react-art",
2037620376
currentDispatcherRef: ReactSharedInternals,
20377-
reconcilerVersion: "19.3.0-www-classic-7f5ea1bf-20251016"
20377+
reconcilerVersion: "19.3.0-www-classic-1324e1bb-20251016"
2037820378
};
2037920379
internals.overrideHookState = overrideHookState;
2038020380
internals.overrideHookStateDeletePath = overrideHookStateDeletePath;
@@ -20409,7 +20409,7 @@ __DEV__ &&
2040920409
exports.Shape = Shape;
2041020410
exports.Surface = Surface;
2041120411
exports.Text = Text;
20412-
exports.version = "19.3.0-www-classic-7f5ea1bf-20251016";
20412+
exports.version = "19.3.0-www-classic-1324e1bb-20251016";
2041320413
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
2041420414
"function" ===
2041520415
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&

0 commit comments

Comments
 (0)