From 48b52d896eb5c83e491facadcf2983374f253c7f Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 17 Oct 2025 16:54:07 -0700 Subject: [PATCH] [compiler] Fix false positive for useMemo reassigning context vars Within a function expresssion local variables use StoreContext, not StoreLocal, so the reassignment check here was firing too often. We should only report an error for variables that are declared outside the function, ie part of its `context`. --- .../src/Validation/ValidateUseMemo.ts | 31 +++++++------ .../reassign-variable-in-usememo.expect.md | 45 +++++++++++++++++++ .../compiler/reassign-variable-in-usememo.js | 12 +++++ 3 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.js 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 05a4b4b91f557..2bccda3a2e930 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateUseMemo.ts @@ -184,25 +184,28 @@ function validateNoContextVariableAssignment( fn: HIRFunction, errors: CompilerError, ): void { + const context = new Set(fn.context.map(place => place.identifier.id)); 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', - }), - ); + if (context.has(value.lvalue.place.identifier.id)) { + 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/reassign-variable-in-usememo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.expect.md new file mode 100644 index 0000000000000..29dc3afcd6fe8 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.expect.md @@ -0,0 +1,45 @@ + +## Input + +```javascript +// @flow +export hook useItemLanguage(items) { + return useMemo(() => { + let language: ?string = null; + items.forEach(item => { + if (item.language != null) { + language = item.language; + } + }); + return language; + }, [items]); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +export function useItemLanguage(items) { + const $ = _c(2); + let language; + if ($[0] !== items) { + language = null; + items.forEach((item) => { + if (item.language != null) { + language = item.language; + } + }); + $[0] = items; + $[1] = language; + } else { + language = $[1]; + } + return language; +} + +``` + +### 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/reassign-variable-in-usememo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.js new file mode 100644 index 0000000000000..4ed89bfc64337 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reassign-variable-in-usememo.js @@ -0,0 +1,12 @@ +// @flow +export hook useItemLanguage(items) { + return useMemo(() => { + let language: ?string = null; + items.forEach(item => { + if (item.language != null) { + language = item.language; + } + }); + return language; + }, [items]); +}