From dc6ca6affcab92e2048bddda39ec836cdb370ea1 Mon Sep 17 00:00:00 2001 From: Dmitrii Troitskii Date: Sat, 2 May 2026 10:53:09 +0000 Subject: [PATCH 1/3] [compiler] fix: preserve-manual-memoization validation should not require stable (non-reactive) values in deps State setters (from useState), dispatch functions (from useReducer), startTransition, and other stable values guaranteed not to change identity across renders should not be required in manual dependency arrays. The Rules of React and the exhaustive-deps lint rule explicitly exempt stable values from dependency lists. However, ValidatePreservedManualMemoization's validateInferredDep was checking ALL inferred dependencies against the user-provided source deps list, including stable non-reactive values. This caused the compiler to skip optimizing components that correctly omitted stable values like state setters from their useCallback/useMemo dependency arrays. Fix: skip the validateInferredDep check when dep.reactive is false AND the identifier is a stable type (isStableType). We gate on !dep.reactive to preserve the existing behavior for edge cases like `const ref = cond ? ref1 : ref2` where a stable-typed identifier happens to be reactive because its value depends on a reactive condition. Fixes: https://github.com/facebook/react/issues/36384 --- .../ValidatePreservedManualMemoization.ts | 17 ++ ...Callback-state-setter-stable-dep.expect.md | 145 ++++++++++++++++++ .../useCallback-state-setter-stable-dep.ts | 37 +++++ 3 files changed, 199 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-stable-dep.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-stable-dep.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts index d39aa307dfb3..662ebb56f8e9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -26,6 +26,7 @@ import { ReactiveValue, ScopeId, SourceLocation, + isStableType, } from '../HIR'; import {Environment} from '../HIR/Environment'; import {printIdentifier, printManualMemoDependency} from '../HIR/PrintHIR'; @@ -233,6 +234,22 @@ function validateInferredDep( env: Environment, memoLocation: SourceLocation, ): void { + /* + * Stable values (state setters, dispatch functions, refs, etc.) that are + * not reactive are guaranteed to never change identity across renders. The + * Rules of React and exhaustive-deps rules do not require them in + * dependency arrays, so the preserve-manual-memoization validation should + * not require them either. Requiring stable, non-reactive values causes + * false-positive skips when user code correctly omits them from a manual + * `useCallback`/`useMemo` deps list. + * + * Note: we only skip non-reactive stable values. Reactive identifiers that + * happen to have a stable type (e.g. `const ref = cond ? ref1 : ref2`) + * still need to appear in the deps array because their identity changes. + */ + if (!dep.reactive && isStableType(dep.identifier)) { + return; + } let normalizedDep: ManualMemoDependency; const maybeNormalizedRoot = temporaries.get(dep.identifier.id); if (maybeNormalizedRoot != null) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-stable-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-stable-dep.expect.md new file mode 100644 index 000000000000..c4b3f57c6e86 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-stable-dep.expect.md @@ -0,0 +1,145 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees +import {useCallback, useState} from 'react'; + +/** + * State setters (and other stable values like dispatch) must not be required + * in manual dependency arrays. The compiler should not reject a component + * because its useCallback omits a state setter from its deps list, since + * state setters are guaranteed stable across renders. + * + * Regression test for: Compiler requires state setter to be added to + * dependency array (github.com/facebook/react/issues/36384) + */ +function Component({data}: {data: Array}) { + const [processed, setProcessed] = useState>([]); + const [count, setCount] = useState(0); + + const handleProcess = useCallback(async () => { + const result = data.map(d => d.trim()); + setProcessed(prev => [...prev, ...result]); + setCount(c => c + 1); + }, [data]); + + return ( +
+ + {count} + {processed.map((p, i) => ( +
{p}
+ ))} +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{data: ['hello ', ' world']}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees +import { useCallback, useState } from "react"; + +/** + * State setters (and other stable values like dispatch) must not be required + * in manual dependency arrays. The compiler should not reject a component + * because its useCallback omits a state setter from its deps list, since + * state setters are guaranteed stable across renders. + * + * Regression test for: Compiler requires state setter to be added to + * dependency array (github.com/facebook/react/issues/36384) + */ +function Component(t0) { + const $ = _c(13); + const { data } = t0; + let t1; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t1 = []; + $[0] = t1; + } else { + t1 = $[0]; + } + const [processed, setProcessed] = useState(t1); + const [count, setCount] = useState(0); + let t2; + if ($[1] !== data) { + t2 = async () => { + const result = data.map(_temp); + setProcessed((prev) => [...prev, ...result]); + setCount(_temp2); + }; + $[1] = data; + $[2] = t2; + } else { + t2 = $[2]; + } + const handleProcess = t2; + let t3; + if ($[3] !== handleProcess) { + t3 = ; + $[3] = handleProcess; + $[4] = t3; + } else { + t3 = $[4]; + } + let t4; + if ($[5] !== count) { + t4 = {count}; + $[5] = count; + $[6] = t4; + } else { + t4 = $[6]; + } + let t5; + if ($[7] !== processed) { + t5 = processed.map(_temp3); + $[7] = processed; + $[8] = t5; + } else { + t5 = $[8]; + } + let t6; + if ($[9] !== t3 || $[10] !== t4 || $[11] !== t5) { + t6 = ( +
+ {t3} + {t4} + {t5} +
+ ); + $[9] = t3; + $[10] = t4; + $[11] = t5; + $[12] = t6; + } else { + t6 = $[12]; + } + return t6; +} +function _temp3(p, i) { + return
{p}
; +} +function _temp2(c) { + return c + 1; +} +function _temp(d) { + return d.trim(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ data: ["hello ", " world"] }], +}; + +``` + +### Eval output +(kind: ok)
0
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-stable-dep.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-stable-dep.ts new file mode 100644 index 000000000000..20b963ff5f19 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-stable-dep.ts @@ -0,0 +1,37 @@ +// @validatePreserveExistingMemoizationGuarantees +import {useCallback, useState} from 'react'; + +/** + * State setters (and other stable values like dispatch) must not be required + * in manual dependency arrays. The compiler should not reject a component + * because its useCallback omits a state setter from its deps list, since + * state setters are guaranteed stable across renders. + * + * Regression test for: Compiler requires state setter to be added to + * dependency array (github.com/facebook/react/issues/36384) + */ +function Component({data}: {data: Array}) { + const [processed, setProcessed] = useState>([]); + const [count, setCount] = useState(0); + + const handleProcess = useCallback(async () => { + const result = data.map(d => d.trim()); + setProcessed(prev => [...prev, ...result]); + setCount(c => c + 1); + }, [data]); + + return ( +
+ + {count} + {processed.map((p, i) => ( +
{p}
+ ))} +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{data: ['hello ', ' world']}], +}; From 14bab8ab9d4fe888c5ab3788cead35b889c7e250 Mon Sep 17 00:00:00 2001 From: Dmitrii Troitskii Date: Wed, 6 May 2026 07:24:02 +0000 Subject: [PATCH 2/3] test: add proper regression fixture for issue #36384 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous test (useCallback-state-setter-stable-dep.ts) did not actually trigger the bug — it compiled successfully even without the fix because the simple pattern doesn't cause the compiler to include state setters in inferred scope deps. This adds a fixture using the exact pattern from the issue playground (JSON.parse of an external value as initial state + useCallback with empty deps that uses a state setter). This pattern DOES trigger the compiler to incorrectly infer setImages as a required dependency, causing it to skip optimization — the bug this PR fixes. Without the fix in the previous commit, this fixture would produce: Compilation Skipped: Existing memoization could not be preserved. The inferred dependency was `setImages`, but the source dependencies were []. --- ...-setter-inferred-dep-issue-36384.expect.md | 120 ++++++++++++++++++ ...k-state-setter-inferred-dep-issue-36384.ts | 44 +++++++ 2 files changed, 164 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.expect.md new file mode 100644 index 000000000000..8264be11c7f9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.expect.md @@ -0,0 +1,120 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees +import {useCallback, useEffect, useState} from 'react'; + +/** + * Regression test for: https://github.com/facebook/react/issues/36384 + * + * The compiler was incorrectly inferring `setImages` (a state setter) as a + * required dependency of the useCallback scope in this specific pattern, + * where JSON.parse of an external value is used as initial state. + * + * Without the fix, the compiler would produce: + * "Compilation Skipped: Existing memoization could not be preserved. + * The inferred dependency was `setImages`, but the source dependencies were []." + * + * This test should compile successfully — state setters are stable values + * and must not be required in manual dependency arrays. + */ + +function getValue() { + return 'true'; +} + +function ImageLibraryPicker() { + const boolString = getValue(); + const [images, setImages] = useState(''); + const [booleanState, setBooleanState] = useState( + JSON.parse(boolString), + ); + + const search = useCallback(() => { + setImages(''); + }, []); + + useEffect(() => { + search(); + }, [search]); + + return null; +} + +export const FIXTURE_ENTRYPOINT = { + fn: ImageLibraryPicker, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees +import { useCallback, useEffect, useState } from "react"; + +/** + * Regression test for: https://github.com/facebook/react/issues/36384 + * + * The compiler was incorrectly inferring `setImages` (a state setter) as a + * required dependency of the useCallback scope in this specific pattern, + * where JSON.parse of an external value is used as initial state. + * + * Without the fix, the compiler would produce: + * "Compilation Skipped: Existing memoization could not be preserved. + * The inferred dependency was `setImages`, but the source dependencies were []." + * + * This test should compile successfully — state setters are stable values + * and must not be required in manual dependency arrays. + */ + +function getValue() { + return "true"; +} + +function ImageLibraryPicker() { + const $ = _c(5); + const boolString = getValue(); + const [, setImages] = useState(""); + useState(JSON.parse(boolString)); + let t0; + if ($[0] !== setImages) { + t0 = () => { + setImages(""); + }; + $[0] = setImages; + $[1] = t0; + } else { + t0 = $[1]; + } + const search = t0; + let t1; + let t2; + if ($[2] !== search) { + t1 = () => { + search(); + }; + t2 = [search]; + $[2] = search; + $[3] = t1; + $[4] = t2; + } else { + t1 = $[3]; + t2 = $[4]; + } + useEffect(t1, t2); + + return null; +} + +export const FIXTURE_ENTRYPOINT = { + fn: ImageLibraryPicker, + params: [], +}; + +``` + +### Eval output +(kind: ok) null \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.ts new file mode 100644 index 000000000000..7253d0b47bf2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.ts @@ -0,0 +1,44 @@ +// @validatePreserveExistingMemoizationGuarantees +import {useCallback, useEffect, useState} from 'react'; + +/** + * Regression test for: https://github.com/facebook/react/issues/36384 + * + * The compiler was incorrectly inferring `setImages` (a state setter) as a + * required dependency of the useCallback scope in this specific pattern, + * where JSON.parse of an external value is used as initial state. + * + * Without the fix, the compiler would produce: + * "Compilation Skipped: Existing memoization could not be preserved. + * The inferred dependency was `setImages`, but the source dependencies were []." + * + * This test should compile successfully — state setters are stable values + * and must not be required in manual dependency arrays. + */ + +function getValue() { + return 'true'; +} + +function ImageLibraryPicker() { + const boolString = getValue(); + const [images, setImages] = useState(''); + const [booleanState, setBooleanState] = useState( + JSON.parse(boolString), + ); + + const search = useCallback(() => { + setImages(''); + }, []); + + useEffect(() => { + search(); + }, [search]); + + return null; +} + +export const FIXTURE_ENTRYPOINT = { + fn: ImageLibraryPicker, + params: [], +}; From 02e8f6842b3d19f443a99df7f2499a6d88fdcb39 Mon Sep 17 00:00:00 2001 From: Dmitrii Troitskii Date: Wed, 6 May 2026 17:16:07 +0000 Subject: [PATCH 3/3] [compiler] root cause fix: exclude stable types (state setters, dispatch) from scope dependencies in PruneNonReactiveDependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit State setters (from useState), dispatch functions (from useReducer), and other stable values guaranteed not to change identity across renders should never appear as scope dependencies. Including them produces unnecessary cache invalidation checks (e.g., `if ($[0] !== setImages)`) and can cause preserve-manual-memoization validation failures when users correctly omit them from manual dep arrays. Root cause: PropagateScopeDependenciesHIR correctly sets dep.reactive=false for stable types, but PruneNonReactiveDependencies only pruned based on the reactive identifier set from collectReactiveIdentifiers. If a stable type's identifier appeared reactive (e.g., captured in a reactive closure), it could survive pruning. Fix: in PruneNonReactiveDependencies.visitScope, also prune deps where !dep.reactive && isStableType(dep.identifier). We guard on !dep.reactive to preserve truly reactive identifiers that happen to have a stable type (e.g., `const ref = cond ? ref1 : ref2` — identity changes with cond). Result: the generated cache checks now correctly omit stable values: Before: `if ($[0] !== setImages) {` (unnecessary dep check) After: `if ($[0] === Symbol.for("react.memo_cache_sentinel")) {` (no deps) Updated fixtures: - preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384 - hoisting-setstate-captured-indirectly-jsx Fixes #36384 --- .../PruneNonReactiveDependencies.ts | 18 ++++++++++++- ...setstate-captured-indirectly-jsx.expect.md | 25 ++++++++----------- ...-setter-inferred-dep-issue-36384.expect.md | 20 +++++++-------- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts index 9bdf15aeb534..5c2fb1b19d95 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts @@ -97,7 +97,23 @@ class Visitor extends ReactiveFunctionVisitor { this.traverseScope(scopeBlock, state); for (const dep of scopeBlock.scope.dependencies) { const isReactive = state.has(dep.identifier.id); - if (!isReactive) { + /* + * Stable values (state setters, dispatch functions, etc.) are guaranteed + * to have a stable identity across renders and should never be scope + * dependencies. Even if they appear reactive due to how they're captured + * in closures, including them as deps produces unnecessary cache checks. + */ + /* + * Stable values (state setters, dispatch functions, etc.) are guaranteed + * to have a stable identity across renders and should never be scope + * dependencies. Even if they appear reactive due to how they're captured + * in closures, including them as deps produces unnecessary cache checks. + * + * Note: we guard on !dep.reactive to preserve reactive identifiers that + * happen to have a stable type (e.g. `const ref = cond ? ref1 : ref2`), + * which DO change identity based on reactive inputs. + */ + if (!isReactive || (!dep.reactive && isStableType(dep.identifier))) { scopeBlock.scope.dependencies.delete(dep); } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-setstate-captured-indirectly-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-setstate-captured-indirectly-jsx.expect.md index 9397518d2d11..f9833e98b3f1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-setstate-captured-indirectly-jsx.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-setstate-captured-indirectly-jsx.expect.md @@ -27,7 +27,7 @@ function useFoo() { ```javascript import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees function useFoo() { - const $ = _c(7); + const $ = _c(4); const onClick = (response) => { setState(DISABLED_FORM); }; @@ -35,34 +35,31 @@ function useFoo() { const [, t0] = useState(); const setState = t0; let t1; - if ($[0] !== setState) { + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t1 = () => { setState(DISABLED_FORM); }; - $[0] = setState; - $[1] = t1; + $[0] = t1; } else { - t1 = $[1]; + t1 = $[0]; } setState; const handleLogout = t1; let t2; - if ($[2] !== handleLogout) { + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { const getComponent = () => handleLogout()} />; t2 = getComponent(); - $[2] = handleLogout; - $[3] = t2; + $[1] = t2; } else { - t2 = $[3]; + t2 = $[1]; } let t3; - if ($[4] !== onClick || $[5] !== t2) { + if ($[2] !== onClick) { t3 = [t2, onClick]; - $[4] = onClick; - $[5] = t2; - $[6] = t3; + $[2] = onClick; + $[3] = t3; } else { - t3 = $[6]; + t3 = $[3]; } return t3; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.expect.md index 8264be11c7f9..62eb618b0290 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-state-setter-inferred-dep-issue-36384.expect.md @@ -75,34 +75,32 @@ function getValue() { } function ImageLibraryPicker() { - const $ = _c(5); + const $ = _c(3); const boolString = getValue(); const [, setImages] = useState(""); useState(JSON.parse(boolString)); let t0; - if ($[0] !== setImages) { + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = () => { setImages(""); }; - $[0] = setImages; - $[1] = t0; + $[0] = t0; } else { - t0 = $[1]; + t0 = $[0]; } const search = t0; let t1; let t2; - if ($[2] !== search) { + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { t1 = () => { search(); }; t2 = [search]; - $[2] = search; - $[3] = t1; - $[4] = t2; + $[1] = t1; + $[2] = t2; } else { - t1 = $[3]; - t2 = $[4]; + t1 = $[1]; + t2 = $[2]; } useEffect(t1, t2);