@samitouri / QOS-React / commits / 7f5ea1bf67

[compiler] More useMemo validation (#34868)

Two additional validations for useMemo: * Disallow reassigning to values declared outside the useMemo callback (always on) * Disallow unused useMemo calls (part of the validateNoVoidUseMemo feature flag, which in turn is off by default) We should probably enable this flag though! --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34868). * #34855 * #34882 * __->__ #34868

Joseph Savona committed Oct 16, 2025 at 13:05 UTC 7f5ea1bf67a1a920919ebe2ae6657bdebe505aa0
8 files changed +182 -11
compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
+3 -3
@@ -458,9 +458,9 @@ export function dropManualMemoization(
458 reason: 'useMemo() callbacks must return a value',
459 description: `This ${
460 manualMemo.loadInstr.value.kind === 'PropertyLoad'
461 - ? 'React.useMemo'
462 - : 'useMemo'
463 - } callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects`,
461 + ? 'React.useMemo()'
462 + : 'useMemo()'
463 + } callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`,
464 suggestions: null,
465 }).withDetails({
466 kind: 'error',
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts
+86 -5
@@ -10,7 +10,16 @@ import {
10 CompilerError,
11 ErrorCategory,
12 } from '../CompilerError';
13 -import {FunctionExpression, HIRFunction, IdentifierId} from '../HIR';
13 +import {
14 + FunctionExpression,
15 + HIRFunction,
16 + IdentifierId,
17 + SourceLocation,
18 +} from '../HIR';
19 +import {
20 + eachInstructionValueOperand,
21 + eachTerminalOperand,
22 +} from '../HIR/visitors';
23 import {Result} from '../Utils/Result';
24
25 export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
@@ -18,8 +27,19 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
27 const useMemos = new Set<IdentifierId>();
28 const react = new Set<IdentifierId>();
29 const functions = new Map<IdentifierId, FunctionExpression>();
30 + const unusedUseMemos = new Map<IdentifierId, SourceLocation>();
31 for (const [, block] of fn.body.blocks) {
32 for (const {lvalue, value} of block.instructions) {
33 + if (unusedUseMemos.size !== 0) {
34 + /**
35 + * Most of the time useMemo results are referenced immediately. Don't bother
36 + * scanning instruction operands for useMemos unless there is an as-yet-unused
37 + * useMemo.
38 + */
39 + for (const operand of eachInstructionValueOperand(value)) {
40 + unusedUseMemos.delete(operand.identifier.id);
41 + }
42 + }
43 switch (value.kind) {
44 case 'LoadGlobal': {
45 if (value.binding.name === 'useMemo') {
@@ -45,10 +65,8 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
65 case 'CallExpression': {
66 // Is the function being called useMemo, with at least 1 argument?
67 const callee =
48 - value.kind === 'CallExpression'
49 - ? value.callee.identifier.id
50 - : value.property.identifier.id;
51 - const isUseMemo = useMemos.has(callee);
68 + value.kind === 'CallExpression' ? value.callee : value.property;
69 + const isUseMemo = useMemos.has(callee.identifier.id);
70 if (!isUseMemo || value.args.length === 0) {
71 continue;
72 }
@@ -104,10 +122,73 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
122 );
123 }
124
125 + validateNoContextVariableAssignment(body.loweredFunc.func, errors);
126 +
127 + if (fn.env.config.validateNoVoidUseMemo) {
128 + unusedUseMemos.set(lvalue.identifier.id, callee.loc);
129 + }
130 break;
131 }
132 }
133 }
134 + if (unusedUseMemos.size !== 0) {
135 + for (const operand of eachTerminalOperand(block.terminal)) {
136 + unusedUseMemos.delete(operand.identifier.id);
137 + }
138 + }
139 + }
140 + if (unusedUseMemos.size !== 0) {
141 + /**
142 + * Basic check for unused memos, where the result of the call is never referenced. This runs
143 + * before DCE so it's more of an AST-level check that something, _anything_, cares about the value.
144 + *
145 + * This is easy to defeat with e.g. `const _ = useMemo(...)` but it at least gives us something to teach.
146 + * Even a DCE-based version could be bypassed with `noop(useMemo(...))`.
147 + */
148 + for (const loc of unusedUseMemos.values()) {
149 + errors.pushDiagnostic(
150 + CompilerDiagnostic.create({
151 + category: ErrorCategory.VoidUseMemo,
152 + reason: 'Unused useMemo()',
153 + description: `This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects`,
154 + suggestions: null,
155 + }).withDetails({
156 + kind: 'error',
157 + loc,
158 + message: 'useMemo() result is unused',
159 + }),
160 + );
161 + }
162 }
163 return errors.asResult();
164 }
165 +
166 +function validateNoContextVariableAssignment(
167 + fn: HIRFunction,
168 + errors: CompilerError,
169 +): void {
170 + for (const block of fn.body.blocks.values()) {
171 + for (const instr of block.instructions) {
172 + const value = instr.value;
173 + switch (value.kind) {
174 + case 'StoreContext': {
175 + errors.pushDiagnostic(
176 + CompilerDiagnostic.create({
177 + category: ErrorCategory.UseMemo,
178 + reason:
179 + 'useMemo() callbacks may not reassign variables declared outside of the callback',
180 + description:
181 + 'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function',
182 + suggestions: null,
183 + }).withDetails({
184 + kind: 'error',
185 + loc: value.lvalue.place.loc,
186 + message: 'Cannot reassign variable',
187 + }),
188 + );
189 + break;
190 + }
191 + }
192 + }
193 + }
194 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.expect.md new
+38
@@ -0,0 +1,38 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component() {
6 + let x;
7 + const y = useMemo(() => {
8 + let z;
9 + x = [];
10 + z = true;
11 + return z;
12 + }, []);
13 + return [x, y];
14 +}
15 +
16 +```
17 +
18 +
19 +## Error
20 +
21 +```
22 +Found 1 error:
23 +
24 +Error: useMemo() callbacks may not reassign variables declared outside of the callback
25 +
26 +useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function.
27 +
28 +error.invalid-reassign-variable-in-usememo.ts:5:4
29 + 3 | const y = useMemo(() => {
30 + 4 | let z;
31 +> 5 | x = [];
32 + | ^ Cannot reassign variable
33 + 6 | z = true;
34 + 7 | return z;
35 + 8 | }, []);
36 +```
37 +
38 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.js new
+10
@@ -0,0 +1,10 @@
1 +function Component() {
2 + let x;
3 + const y = useMemo(() => {
4 + let z;
5 + x = [];
6 + z = true;
7 + return z;
8 + }, []);
9 + return [x, y];
10 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md new
+35
@@ -0,0 +1,35 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @validateNoVoidUseMemo
6 +function Component() {
7 + useMemo(() => {
8 + return [];
9 + }, []);
10 + return <div />;
11 +}
12 +
13 +```
14 +
15 +
16 +## Error
17 +
18 +```
19 +Found 1 error:
20 +
21 +Error: Unused useMemo()
22 +
23 +This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects.
24 +
25 +error.invalid-unused-usememo.ts:3:2
26 + 1 | // @validateNoVoidUseMemo
27 + 2 | function Component() {
28 +> 3 | useMemo(() => {
29 + | ^^^^^^^ useMemo() result is unused
30 + 4 | return [];
31 + 5 | }, []);
32 + 6 | return <div />;
33 +```
34 +
35 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.js new
+7
@@ -0,0 +1,7 @@
1 +// @validateNoVoidUseMemo
2 +function Component() {
3 + useMemo(() => {
4 + return [];
5 + }, []);
6 + return <div />;
7 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md
+2 -2
@@ -28,7 +28,7 @@ Found 2 errors:
28
29 Error: useMemo() callbacks must return a value
30
31 -This useMemo callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects.
31 +This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects.
32
33 error.useMemo-no-return-value.ts:3:16
34 1 | // @validateNoVoidUseMemo
@@ -45,7 +45,7 @@ error.useMemo-no-return-value.ts:3:16
45
46 Error: useMemo() callbacks must return a value
47
48 -This React.useMemo callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects.
48 +This React.useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects.
49
50 error.useMemo-no-return-value.ts:6:17
51 4 | console.log('computing');
compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts
+1 -1
@@ -120,7 +120,7 @@ testRule('plugin-recommended', TestRecommendedRules, {
120
121 return <Child x={state} />;
122 }`,
123 - errors: [makeTestCaseError('useMemo() callbacks must return a value')],
123 + errors: [makeTestCaseError('Unused useMemo()')],
124 },
125 {
126 name: 'Pipeline errors are reported',