From 5293f2ce62dfb31bf93780edc2b763d602359f21 Mon Sep 17 00:00:00 2001 From: Kioumars Rahimi Date: Sun, 19 Oct 2025 14:13:46 +0330 Subject: [PATCH 1/3] [Compiler] Fix incorrect closure hoisting causing runtime errors (#34901) The React Compiler incorrectly outlines closure-scoped functions to the top level when it detects they have no context variables from the immediate component scope. This breaks lexical scoping for nested components that capture variables from parent scopes. The issue occurs when: 1. A component contains a nested function component 2. That nested component has an arrow function 3. The arrow function captures a variable from the parent component The `outlineFunctions` optimization only checked `context.length === 0`, which verifies variables from the immediate component scope, but doesn't account for variables from outer parent scopes accessed via LoadContext instructions. Fix: Added `hasUnaccountedOuterScopeReferences()` helper that checks for LoadContext instructions in the function's HIR. If any exist, the function cannot be safely outlined as it depends on runtime context. Test: Added closure-hoisting-bug.js fixture that reproduces the issue and verifies the fix. --- .../src/Optimization/OutlineFunctions.ts | 36 +++++++- .../compiler/closure-hoisting-bug.expect.md | 86 +++++++++++++++++++ .../fixtures/compiler/closure-hoisting-bug.js | 29 +++++++ 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/closure-hoisting-bug.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/closure-hoisting-bug.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts index 6ab0a811ff5d..1fd904aa3545 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts @@ -7,6 +7,36 @@ import {HIRFunction, IdentifierId} from '../HIR'; +/** + * Checks if a function has any references to variables from outer scopes + * (beyond the explicitly tracked context array). This includes LoadContext + * instructions which indicate the function is accessing variables that are + * not passed as parameters or declared locally. + */ +function hasUnaccountedOuterScopeReferences(fn: HIRFunction): boolean { + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + // Check if this instruction loads from outer scope context + if (instr.value.kind === 'LoadContext') { + // LoadContext means we're accessing a variable from an outer scope + // that's not in the context array. This function cannot be safely outlined. + return true; + } + + // Recursively check nested functions + if ( + instr.value.kind === 'FunctionExpression' || + instr.value.kind === 'ObjectMethod' + ) { + if (hasUnaccountedOuterScopeReferences(instr.value.loweredFunc.func)) { + return true; + } + } + } + } + return false; +} + export function outlineFunctions( fn: HIRFunction, fbtOperands: Set, @@ -27,7 +57,11 @@ export function outlineFunctions( value.loweredFunc.func.context.length === 0 && // TODO: handle outlining named functions value.loweredFunc.func.id === null && - !fbtOperands.has(lvalue.identifier.id) + !fbtOperands.has(lvalue.identifier.id) && + // FIXED: Also check that the function doesn't have any LoadContext instructions + // which would indicate it's accessing variables from outer scopes not tracked + // in the context array. See https://github.com/facebook/react/issues/34901 + !hasUnaccountedOuterScopeReferences(value.loweredFunc.func) ) { const loweredFunc = value.loweredFunc.func; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/closure-hoisting-bug.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/closure-hoisting-bug.expect.md new file mode 100644 index 000000000000..4448020547ff --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/closure-hoisting-bug.expect.md @@ -0,0 +1,86 @@ + +## Input + +```javascript +// @compilationMode(infer) +// Regression test for https://github.com/facebook/react/issues/34901 +// The compiler should NOT outline functions that capture variables from their closure. +// In this case, `() => store` captures `store` from the outer scope and should not +// be hoisted to a top-level function because `store` would be undefined. + +class SomeStore { + test = 'hello'; +} + +function useLocalObservable(fn) { + return fn(); +} + +export function createSomething() { + const store = new SomeStore(); + + const Cmp = () => { + const observedStore = useLocalObservable(() => store); + return
{observedStore.test}
; + }; + + return Cmp; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createSomething, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +class SomeStore { + test = "hello"; +} + +function useLocalObservable(fn) { + return fn(); +} + +export function createSomething() { + const $ = _c(2); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const store = new SomeStore(); + + const Cmp = () => { + const $ = _c(2); + const observedStore = useLocalObservable(() => store); + let t0; + if ($[0] !== observedStore.test) { + t0 =
{observedStore.test}
; + $[0] = observedStore.test; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; + }; + + t0 = Cmp; + $[0] = t0; + $[1] = store; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createSomething, + params: [], +}; + +``` + +### Eval output +(kind: ok)
hello
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/closure-hoisting-bug.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/closure-hoisting-bug.js new file mode 100644 index 000000000000..8f23e983c138 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/closure-hoisting-bug.js @@ -0,0 +1,29 @@ +// @compilationMode(infer) +// Regression test for https://github.com/facebook/react/issues/34901 +// The compiler should NOT outline functions that capture variables from their closure. +// In this case, `() => store` captures `store` from the outer scope and should not +// be hoisted to a top-level function because `store` would be undefined. + +class SomeStore { + test = 'hello'; +} + +function useLocalObservable(fn) { + return fn(); +} + +export function createSomething() { + const store = new SomeStore(); + + const Cmp = () => { + const observedStore = useLocalObservable(() => store); + return
{observedStore.test}
; + }; + + return Cmp; +} + +export const FIXTURE_ENTRYPOINT = { + fn: createSomething, + params: [], +}; From 0d0035b38de07ea86d1b3dab01ec5e3a9405e930 Mon Sep 17 00:00:00 2001 From: Kioumars Rahimi Date: Sun, 19 Oct 2025 14:55:21 +0330 Subject: [PATCH 2/3] Resolve closure-hoisting-bug-34901 --- packages/shared/ReactVersion.js | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/packages/shared/ReactVersion.js b/packages/shared/ReactVersion.js index bd5fa23ca26b..346684575077 100644 --- a/packages/shared/ReactVersion.js +++ b/packages/shared/ReactVersion.js @@ -1,15 +1 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -// TODO: this is special because it gets imported during build. -// -// It exists as a placeholder so that DevTools can support work tag changes between releases. -// When we next publish a release, update the matching TODO in backend/renderer.js -// TODO: This module is used both by the release scripts and to expose a version -// at runtime. We should instead inject the version number as part of the build -// process, and use the ReactVersions.js module as the single source of truth. -export default '19.3.0'; +export default '19.3.0-canary-40c7a7f6-20251018'; From 64be1f63e4ec874feb07e377c2942b9cd0d87023 Mon Sep 17 00:00:00 2001 From: Kioumars Rahimi Date: Mon, 20 Oct 2025 00:13:44 +0330 Subject: [PATCH 3/3] [Optimization] Refactor context variable access check and update function naming [Version] Update React version to 19.3.0 --- .../src/Optimization/OutlineFunctions.ts | 38 +++++++++++-------- packages/shared/ReactVersion.js | 2 +- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts index 1fd904aa3545..4ea1d4a93093 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts @@ -8,27 +8,36 @@ import {HIRFunction, IdentifierId} from '../HIR'; /** - * Checks if a function has any references to variables from outer scopes - * (beyond the explicitly tracked context array). This includes LoadContext - * instructions which indicate the function is accessing variables that are - * not passed as parameters or declared locally. + * Checks if a function accesses context variables that are not explicitly + * tracked in its context array. Context variables are those that are both + * accessed within a function expression and reassigned somewhere in the + * outer scope. */ -function hasUnaccountedOuterScopeReferences(fn: HIRFunction): boolean { +function accessesOuterContextVariables(fn: HIRFunction): boolean { + const contextIdentifiers = new Set( + fn.context.map(place => place.identifier.id), + ); + for (const [, block] of fn.body.blocks) { for (const instr of block.instructions) { - // Check if this instruction loads from outer scope context - if (instr.value.kind === 'LoadContext') { - // LoadContext means we're accessing a variable from an outer scope - // that's not in the context array. This function cannot be safely outlined. - return true; + if ( + instr.value.kind === 'LoadContext' || + instr.value.kind === 'StoreContext' + ) { + const place = + instr.value.kind === 'LoadContext' + ? instr.value.place + : instr.value.lvalue.place; + if (!contextIdentifiers.has(place.identifier.id)) { + return true; + } } - // Recursively check nested functions if ( instr.value.kind === 'FunctionExpression' || instr.value.kind === 'ObjectMethod' ) { - if (hasUnaccountedOuterScopeReferences(instr.value.loweredFunc.func)) { + if (accessesOuterContextVariables(instr.value.loweredFunc.func)) { return true; } } @@ -58,10 +67,7 @@ export function outlineFunctions( // TODO: handle outlining named functions value.loweredFunc.func.id === null && !fbtOperands.has(lvalue.identifier.id) && - // FIXED: Also check that the function doesn't have any LoadContext instructions - // which would indicate it's accessing variables from outer scopes not tracked - // in the context array. See https://github.com/facebook/react/issues/34901 - !hasUnaccountedOuterScopeReferences(value.loweredFunc.func) + !accessesOuterContextVariables(value.loweredFunc.func) ) { const loweredFunc = value.loweredFunc.func; diff --git a/packages/shared/ReactVersion.js b/packages/shared/ReactVersion.js index 346684575077..84ed54f8df2e 100644 --- a/packages/shared/ReactVersion.js +++ b/packages/shared/ReactVersion.js @@ -1 +1 @@ -export default '19.3.0-canary-40c7a7f6-20251018'; +export default '19.3.0';