From 7f5ea1bf67a1a920919ebe2ae6657bdebe505aa0 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Thu, 16 Oct 2025 13:05:18 -0700 Subject: [PATCH 1/2] [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 --- .../src/Inference/DropManualMemoization.ts | 6 +- .../src/Validation/ValidateUseMemo.ts | 91 ++++++++++++++++++- ...lid-reassign-variable-in-usememo.expect.md | 38 ++++++++ ...or.invalid-reassign-variable-in-usememo.js | 10 ++ .../error.invalid-unused-usememo.expect.md | 35 +++++++ .../compiler/error.invalid-unused-usememo.js | 7 ++ .../error.useMemo-no-return-value.expect.md | 4 +- .../__tests__/PluginTest-test.ts | 2 +- 8 files changed, 182 insertions(+), 11 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts index f86747618dd..d31cd1446b9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts @@ -458,9 +458,9 @@ export function dropManualMemoization( reason: 'useMemo() callbacks must return a value', description: `This ${ manualMemo.loadInstr.value.kind === 'PropertyLoad' - ? 'React.useMemo' - : 'useMemo' - } callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects`, + ? 'React.useMemo()' + : 'useMemo()' + } callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`, suggestions: null, }).withDetails({ kind: 'error', diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts index de74af93b6d..0fdfeaab3ea 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts @@ -10,7 +10,16 @@ import { CompilerError, ErrorCategory, } from '../CompilerError'; -import {FunctionExpression, HIRFunction, IdentifierId} from '../HIR'; +import { + FunctionExpression, + HIRFunction, + IdentifierId, + SourceLocation, +} from '../HIR'; +import { + eachInstructionValueOperand, + eachTerminalOperand, +} from '../HIR/visitors'; import {Result} from '../Utils/Result'; export function validateUseMemo(fn: HIRFunction): Result { @@ -18,8 +27,19 @@ export function validateUseMemo(fn: HIRFunction): Result { const useMemos = new Set(); const react = new Set(); const functions = new Map(); + const unusedUseMemos = new Map(); for (const [, block] of fn.body.blocks) { for (const {lvalue, value} of block.instructions) { + if (unusedUseMemos.size !== 0) { + /** + * Most of the time useMemo results are referenced immediately. Don't bother + * scanning instruction operands for useMemos unless there is an as-yet-unused + * useMemo. + */ + for (const operand of eachInstructionValueOperand(value)) { + unusedUseMemos.delete(operand.identifier.id); + } + } switch (value.kind) { case 'LoadGlobal': { if (value.binding.name === 'useMemo') { @@ -45,10 +65,8 @@ export function validateUseMemo(fn: HIRFunction): Result { case 'CallExpression': { // Is the function being called useMemo, with at least 1 argument? const callee = - value.kind === 'CallExpression' - ? value.callee.identifier.id - : value.property.identifier.id; - const isUseMemo = useMemos.has(callee); + value.kind === 'CallExpression' ? value.callee : value.property; + const isUseMemo = useMemos.has(callee.identifier.id); if (!isUseMemo || value.args.length === 0) { continue; } @@ -104,10 +122,73 @@ export function validateUseMemo(fn: HIRFunction): Result { ); } + validateNoContextVariableAssignment(body.loweredFunc.func, errors); + + if (fn.env.config.validateNoVoidUseMemo) { + unusedUseMemos.set(lvalue.identifier.id, callee.loc); + } break; } } } + if (unusedUseMemos.size !== 0) { + for (const operand of eachTerminalOperand(block.terminal)) { + unusedUseMemos.delete(operand.identifier.id); + } + } + } + if (unusedUseMemos.size !== 0) { + /** + * Basic check for unused memos, where the result of the call is never referenced. This runs + * before DCE so it's more of an AST-level check that something, _anything_, cares about the value. + * + * This is easy to defeat with e.g. `const _ = useMemo(...)` but it at least gives us something to teach. + * Even a DCE-based version could be bypassed with `noop(useMemo(...))`. + */ + for (const loc of unusedUseMemos.values()) { + errors.pushDiagnostic( + CompilerDiagnostic.create({ + category: ErrorCategory.VoidUseMemo, + reason: 'Unused useMemo()', + description: `This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects`, + suggestions: null, + }).withDetails({ + kind: 'error', + loc, + message: 'useMemo() result is unused', + }), + ); + } } return errors.asResult(); } + +function validateNoContextVariableAssignment( + fn: HIRFunction, + errors: CompilerError, +): void { + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + const value = instr.value; + switch (value.kind) { + case 'StoreContext': { + errors.pushDiagnostic( + CompilerDiagnostic.create({ + category: ErrorCategory.UseMemo, + reason: + 'useMemo() callbacks may not reassign variables declared outside of the callback', + description: + 'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function', + suggestions: null, + }).withDetails({ + kind: 'error', + loc: value.lvalue.place.loc, + message: 'Cannot reassign variable', + }), + ); + break; + } + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.expect.md new file mode 100644 index 00000000000..ba35168bcfe --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.expect.md @@ -0,0 +1,38 @@ + +## Input + +```javascript +function Component() { + let x; + const y = useMemo(() => { + let z; + x = []; + z = true; + return z; + }, []); + return [x, y]; +} + +``` + + +## Error + +``` +Found 1 error: + +Error: useMemo() callbacks may not reassign variables declared outside of the callback + +useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function. + +error.invalid-reassign-variable-in-usememo.ts:5:4 + 3 | const y = useMemo(() => { + 4 | let z; +> 5 | x = []; + | ^ Cannot reassign variable + 6 | z = true; + 7 | return z; + 8 | }, []); +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.js new file mode 100644 index 00000000000..885ba4b52d5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-variable-in-usememo.js @@ -0,0 +1,10 @@ +function Component() { + let x; + const y = useMemo(() => { + let z; + x = []; + z = true; + return z; + }, []); + return [x, y]; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md new file mode 100644 index 00000000000..3c84a0dda33 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md @@ -0,0 +1,35 @@ + +## Input + +```javascript +// @validateNoVoidUseMemo +function Component() { + useMemo(() => { + return []; + }, []); + return
; +} + +``` + + +## Error + +``` +Found 1 error: + +Error: Unused useMemo() + +This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects. + +error.invalid-unused-usememo.ts:3:2 + 1 | // @validateNoVoidUseMemo + 2 | function Component() { +> 3 | useMemo(() => { + | ^^^^^^^ useMemo() result is unused + 4 | return []; + 5 | }, []); + 6 | return
; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.js new file mode 100644 index 00000000000..1983cd061a6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.js @@ -0,0 +1,7 @@ +// @validateNoVoidUseMemo +function Component() { + useMemo(() => { + return []; + }, []); + return
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md index fb595964066..9b62c5364b6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md @@ -28,7 +28,7 @@ Found 2 errors: Error: useMemo() callbacks must return a value -This useMemo callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects. +This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects. error.useMemo-no-return-value.ts:3:16 1 | // @validateNoVoidUseMemo @@ -45,7 +45,7 @@ error.useMemo-no-return-value.ts:3:16 Error: useMemo() callbacks must return a value -This React.useMemo callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects. +This React.useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects. error.useMemo-no-return-value.ts:6:17 4 | console.log('computing'); diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts index 6efd069aafa..89eb2ab22f6 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts @@ -120,7 +120,7 @@ testRule('plugin-recommended', TestRecommendedRules, { return ; }`, - errors: [makeTestCaseError('useMemo() callbacks must return a value')], + errors: [makeTestCaseError('Unused useMemo()')], }, { name: 'Pipeline errors are reported', From 1324e1bb1f867e8b2108ca52a1d4e2d4ef56d2d9 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Thu, 16 Oct 2025 13:08:57 -0700 Subject: [PATCH 2/2] [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 --- .../src/CompilerError.ts | 2 +- .../src/HIR/Environment.ts | 2 +- .../src/Inference/DropManualMemoization.ts | 48 -------------- .../src/Validation/ValidateUseMemo.ts | 37 ++++++++++- .../error.invalid-unused-usememo.expect.md | 35 ---------- .../error.useMemo-no-return-value.expect.md | 64 ------------------- .../compiler/invalid-unused-usememo.expect.md | 41 ++++++++++++ ...d-usememo.js => invalid-unused-usememo.js} | 2 +- .../invalid-useMemo-no-return-value.expect.md | 59 +++++++++++++++++ ....js => invalid-useMemo-no-return-value.js} | 2 +- .../invalid-useMemo-return-empty.expect.md | 33 ++++++++++ ...pty.js => invalid-useMemo-return-empty.js} | 1 + .../fixtures/compiler/repro.expect.md | 3 +- .../src/__tests__/fixtures/compiler/repro.js | 1 + .../compiler/useMemo-nested-ifs.expect.md | 11 +++- .../fixtures/compiler/useMemo-nested-ifs.js | 1 + .../compiler/useMemo-return-empty.expect.md | 22 ------- .../__tests__/PluginTest-test.ts | 10 ++- 18 files changed, 195 insertions(+), 179 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{error.invalid-unused-usememo.js => invalid-unused-usememo.js} (67%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-no-return-value.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{error.useMemo-no-return-value.js => invalid-useMemo-no-return-value.js} (85%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-return-empty.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{useMemo-return-empty.js => invalid-useMemo-return-empty.js} (82%) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index 1c077edd8d8..3c131ea653e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -988,7 +988,7 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule { severity: ErrorSeverity.Error, name: 'void-use-memo', description: - 'Validates that useMemos always return a value. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.', + '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.', preset: LintRulePreset.RecommendedLatest, }; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index a31ef1c336e..4ec4a4a795f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -659,7 +659,7 @@ export const EnvironmentConfigSchema = z.object({ * Invalid: * useMemo(() => { ... }, [...]); */ - validateNoVoidUseMemo: z.boolean().default(false), + validateNoVoidUseMemo: z.boolean().default(true), /** * Validates that Components/Hooks are always defined at module level. This prevents scope diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts index d31cd1446b9..0876424e28c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts @@ -438,40 +438,6 @@ export function dropManualMemoization( continue; } - /** - * Bailout on void return useMemos. This is an anti-pattern where code might be using - * useMemo like useEffect: running arbirtary side-effects synced to changes in specific - * values. - */ - if ( - func.env.config.validateNoVoidUseMemo && - manualMemo.kind === 'useMemo' - ) { - const funcToCheck = sidemap.functions.get( - fnPlace.identifier.id, - )?.value; - if (funcToCheck !== undefined && funcToCheck.loweredFunc.func) { - if (!hasNonVoidReturn(funcToCheck.loweredFunc.func)) { - errors.pushDiagnostic( - CompilerDiagnostic.create({ - category: ErrorCategory.VoidUseMemo, - reason: 'useMemo() callbacks must return a value', - description: `This ${ - manualMemo.loadInstr.value.kind === 'PropertyLoad' - ? 'React.useMemo()' - : 'useMemo()' - } callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`, - suggestions: null, - }).withDetails({ - kind: 'error', - loc: instr.value.loc, - message: 'useMemo() callbacks must return a value', - }), - ); - } - } - } - instr.value = getManualMemoizationReplacement( fnPlace, instr.value.loc, @@ -629,17 +595,3 @@ function findOptionalPlaces(fn: HIRFunction): Set { } return optionals; } - -function hasNonVoidReturn(func: HIRFunction): boolean { - for (const [, block] of func.body.blocks) { - if (block.terminal.kind === 'return') { - if ( - block.terminal.returnVariant === 'Explicit' || - block.terminal.returnVariant === 'Implicit' - ) { - return true; - } - } - } - return false; -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts index 0fdfeaab3ea..05a4b4b91f5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts @@ -24,6 +24,7 @@ import {Result} from '../Utils/Result'; export function validateUseMemo(fn: HIRFunction): Result { const errors = new CompilerError(); + const voidMemoErrors = new CompilerError(); const useMemos = new Set(); const react = new Set(); const functions = new Map(); @@ -125,7 +126,22 @@ export function validateUseMemo(fn: HIRFunction): Result { validateNoContextVariableAssignment(body.loweredFunc.func, errors); if (fn.env.config.validateNoVoidUseMemo) { - unusedUseMemos.set(lvalue.identifier.id, callee.loc); + if (!hasNonVoidReturn(body.loweredFunc.func)) { + voidMemoErrors.pushDiagnostic( + CompilerDiagnostic.create({ + category: ErrorCategory.VoidUseMemo, + reason: 'useMemo() callbacks must return a value', + description: `This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`, + suggestions: null, + }).withDetails({ + kind: 'error', + loc: body.loc, + message: 'useMemo() callbacks must return a value', + }), + ); + } else { + unusedUseMemos.set(lvalue.identifier.id, callee.loc); + } } break; } @@ -146,10 +162,10 @@ export function validateUseMemo(fn: HIRFunction): Result { * Even a DCE-based version could be bypassed with `noop(useMemo(...))`. */ for (const loc of unusedUseMemos.values()) { - errors.pushDiagnostic( + voidMemoErrors.pushDiagnostic( CompilerDiagnostic.create({ category: ErrorCategory.VoidUseMemo, - reason: 'Unused useMemo()', + reason: 'useMemo() result is unused', description: `This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects`, suggestions: null, }).withDetails({ @@ -160,6 +176,7 @@ export function validateUseMemo(fn: HIRFunction): Result { ); } } + fn.env.logErrors(voidMemoErrors.asResult()); return errors.asResult(); } @@ -192,3 +209,17 @@ function validateNoContextVariableAssignment( } } } + +function hasNonVoidReturn(func: HIRFunction): boolean { + for (const [, block] of func.body.blocks) { + if (block.terminal.kind === 'return') { + if ( + block.terminal.returnVariant === 'Explicit' || + block.terminal.returnVariant === 'Implicit' + ) { + return true; + } + } + } + return false; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md deleted file mode 100644 index 3c84a0dda33..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md +++ /dev/null @@ -1,35 +0,0 @@ - -## Input - -```javascript -// @validateNoVoidUseMemo -function Component() { - useMemo(() => { - return []; - }, []); - return
; -} - -``` - - -## Error - -``` -Found 1 error: - -Error: Unused useMemo() - -This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects. - -error.invalid-unused-usememo.ts:3:2 - 1 | // @validateNoVoidUseMemo - 2 | function Component() { -> 3 | useMemo(() => { - | ^^^^^^^ useMemo() result is unused - 4 | return []; - 5 | }, []); - 6 | return
; -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md deleted file mode 100644 index 9b62c5364b6..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md +++ /dev/null @@ -1,64 +0,0 @@ - -## Input - -```javascript -// @validateNoVoidUseMemo -function Component() { - const value = useMemo(() => { - console.log('computing'); - }, []); - const value2 = React.useMemo(() => { - console.log('computing'); - }, []); - return ( -
- {value} - {value2} -
- ); -} - -``` - - -## Error - -``` -Found 2 errors: - -Error: useMemo() callbacks must return a value - -This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects. - -error.useMemo-no-return-value.ts:3:16 - 1 | // @validateNoVoidUseMemo - 2 | function Component() { -> 3 | const value = useMemo(() => { - | ^^^^^^^^^^^^^^^ -> 4 | console.log('computing'); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 5 | }, []); - | ^^^^^^^^^ useMemo() callbacks must return a value - 6 | const value2 = React.useMemo(() => { - 7 | console.log('computing'); - 8 | }, []); - -Error: useMemo() callbacks must return a value - -This React.useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects. - -error.useMemo-no-return-value.ts:6:17 - 4 | console.log('computing'); - 5 | }, []); -> 6 | const value2 = React.useMemo(() => { - | ^^^^^^^^^^^^^^^^^^^^^ -> 7 | console.log('computing'); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 8 | }, []); - | ^^^^^^^^^ useMemo() callbacks must return a value - 9 | return ( - 10 |
- 11 | {value} -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.expect.md new file mode 100644 index 00000000000..a2aba4c7b91 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.expect.md @@ -0,0 +1,41 @@ + +## Input + +```javascript +// @validateNoVoidUseMemo @loggerTestOnly +function Component() { + useMemo(() => { + return []; + }, []); + return
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo @loggerTestOnly +function Component() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"category":"VoidUseMemo","reason":"useMemo() result is unused","description":"This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":3,"column":2,"index":67},"end":{"line":3,"column":9,"index":74},"filename":"invalid-unused-usememo.ts","identifierName":"useMemo"},"message":"useMemo() result is unused"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":42},"end":{"line":7,"column":1,"index":127},"filename":"invalid-unused-usememo.ts"},"fnName":"Component","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.js similarity index 67% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.js index 1983cd061a6..e254424949b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.js @@ -1,4 +1,4 @@ -// @validateNoVoidUseMemo +// @validateNoVoidUseMemo @loggerTestOnly function Component() { useMemo(() => { return []; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-no-return-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-no-return-value.expect.md new file mode 100644 index 00000000000..24e62dad2be --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-no-return-value.expect.md @@ -0,0 +1,59 @@ + +## Input + +```javascript +// @validateNoVoidUseMemo @loggerTestOnly +function Component() { + const value = useMemo(() => { + console.log('computing'); + }, []); + const value2 = React.useMemo(() => { + console.log('computing'); + }, []); + return ( +
+ {value} + {value2} +
+ ); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo @loggerTestOnly +function Component() { + const $ = _c(1); + + console.log("computing"); + + console.log("computing"); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = ( +
+ {undefined} + {undefined} +
+ ); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"category":"VoidUseMemo","reason":"useMemo() callbacks must return a value","description":"This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":3,"column":24,"index":89},"end":{"line":5,"column":3,"index":130},"filename":"invalid-useMemo-no-return-value.ts"},"message":"useMemo() callbacks must return a value"}]}},"fnLoc":null} +{"kind":"CompileError","detail":{"options":{"category":"VoidUseMemo","reason":"useMemo() callbacks must return a value","description":"This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":6,"column":31,"index":168},"end":{"line":8,"column":3,"index":209},"filename":"invalid-useMemo-no-return-value.ts"},"message":"useMemo() callbacks must return a value"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":42},"end":{"line":15,"column":1,"index":283},"filename":"invalid-useMemo-no-return-value.ts"},"fnName":"Component","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0} +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-no-return-value.js similarity index 85% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-no-return-value.js index 0ce35e12f4f..781560fefa9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-no-return-value.js @@ -1,4 +1,4 @@ -// @validateNoVoidUseMemo +// @validateNoVoidUseMemo @loggerTestOnly function Component() { const value = useMemo(() => { console.log('computing'); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-return-empty.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-return-empty.expect.md new file mode 100644 index 00000000000..c6737d78a1b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-return-empty.expect.md @@ -0,0 +1,33 @@ + +## Input + +```javascript +// @loggerTestOnly +function component(a) { + let x = useMemo(() => { + mutate(a); + }, []); + return x; +} + +``` + +## Code + +```javascript +// @loggerTestOnly +function component(a) { + mutate(a); +} + +``` + +## Logs + +``` +{"kind":"CompileError","detail":{"options":{"category":"VoidUseMemo","reason":"useMemo() callbacks must return a value","description":"This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":3,"column":18,"index":61},"end":{"line":5,"column":3,"index":87},"filename":"invalid-useMemo-return-empty.ts"},"message":"useMemo() callbacks must return a value"}]}},"fnLoc":null} +{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":19},"end":{"line":7,"column":1,"index":107},"filename":"invalid-useMemo-return-empty.ts"},"fnName":"component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":1,"prunedMemoValues":0} +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-return-empty.js similarity index 82% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-return-empty.js index 0d9b54d989b..d70bd138113 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-useMemo-return-empty.js @@ -1,3 +1,4 @@ +// @loggerTestOnly function component(a) { let x = useMemo(() => { mutate(a); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md index d3fe3045c02..9ca367e8a8e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @validateNoVoidUseMemo:false function Component(props) { const item = props.item; const thumbnails = []; @@ -22,7 +23,7 @@ function Component(props) { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo:false function Component(props) { const $ = _c(6); const item = props.item; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.js index 9f5d7852668..de3026e4e58 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.js @@ -1,3 +1,4 @@ +// @validateNoVoidUseMemo:false function Component(props) { const item = props.item; const thumbnails = []; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md index f7b3605f4d5..e0525518c96 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md @@ -6,6 +6,7 @@ function Component(props) { const x = useMemo(() => { if (props.cond) { if (props.cond) { + return props.value; } } }, [props.cond]); @@ -24,10 +25,18 @@ export const FIXTURE_ENTRYPOINT = { ```javascript function Component(props) { - if (props.cond) { + let t0; + bb0: { if (props.cond) { + if (props.cond) { + t0 = props.value; + break bb0; + } } + t0 = undefined; } + const x = t0; + return x; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.js index f9b329e1290..d122dcc7a3e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.js @@ -2,6 +2,7 @@ function Component(props) { const x = useMemo(() => { if (props.cond) { if (props.cond) { + return props.value; } } }, [props.cond]); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md deleted file mode 100644 index be20ee39bd4..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md +++ /dev/null @@ -1,22 +0,0 @@ - -## Input - -```javascript -function component(a) { - let x = useMemo(() => { - mutate(a); - }, []); - return x; -} - -``` - -## Code - -```javascript -function component(a) { - mutate(a); -} - -``` - \ No newline at end of file diff --git a/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts b/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts index 89eb2ab22f6..e0063bc0a2c 100644 --- a/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts +++ b/compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts @@ -120,7 +120,15 @@ testRule('plugin-recommended', TestRecommendedRules, { return ; }`, - errors: [makeTestCaseError('Unused useMemo()')], + errors: [ + makeTestCaseError('useMemo() callbacks must return a value'), + makeTestCaseError( + 'Calling setState from useMemo may trigger an infinite loop', + ), + makeTestCaseError( + 'Calling setState from useMemo may trigger an infinite loop', + ), + ], }, { name: 'Pipeline errors are reported',