From 4dcaca490c0672949106f76559be18a7e7e238c1 Mon Sep 17 00:00:00 2001 From: Bobadu Date: Fri, 27 Mar 2026 13:28:29 +0100 Subject: [PATCH 1/4] Add test fixtures for object memoization inconsistency (#36151) --- ...memoization-object-fn-expression.expect.md | 70 ++++++++++++++++ ...istent-memoization-object-fn-expression.js | 15 ++++ ...oization-object-method-shorthand.expect.md | 57 +++++++++++++ ...ent-memoization-object-method-shorthand.js | 15 ++++ ...sistent-memoization-object-mixed.expect.md | 80 +++++++++++++++++++ ...g-inconsistent-memoization-object-mixed.js | 18 +++++ 6 files changed, 255 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-method-shorthand.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-method-shorthand.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.expect.md new file mode 100644 index 000000000000..38508c2ad710 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.expect.md @@ -0,0 +1,70 @@ + +## Input + +```javascript +function Component({a, b}) { + return { + test1: () => { + console.log(a); + }, + test2: function () { + console.log(b); + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 1, b: 2}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(t0) { + const $ = _c(7); + const { a, b } = t0; + let t1; + if ($[0] !== a) { + t1 = () => { + console.log(a); + }; + $[0] = a; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== b) { + t2 = function () { + console.log(b); + }; + $[2] = b; + $[3] = t2; + } else { + t2 = $[3]; + } + let t3; + if ($[4] !== t1 || $[5] !== t2) { + t3 = { test1: t1, test2: t2 }; + $[4] = t1; + $[5] = t2; + $[6] = t3; + } else { + t3 = $[6]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 1, b: 2 }], +}; + +``` + +### Eval output +(kind: ok) {"test1":"[[ function params=0 ]]","test2":"[[ function params=0 ]]"} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.js new file mode 100644 index 000000000000..c26bd857db2f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.js @@ -0,0 +1,15 @@ +function Component({a, b}) { + return { + test1: () => { + console.log(a); + }, + test2: function () { + console.log(b); + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 1, b: 2}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-method-shorthand.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-method-shorthand.expect.md new file mode 100644 index 000000000000..d00facc9385c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-method-shorthand.expect.md @@ -0,0 +1,57 @@ + +## Input + +```javascript +function Component({a, b}) { + return { + test1() { + console.log(a); + }, + test2() { + console.log(b); + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 1, b: 2}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(t0) { + const $ = _c(3); + const { a, b } = t0; + let t1; + if ($[0] !== a || $[1] !== b) { + t1 = { + test1() { + console.log(a); + }, + test2() { + console.log(b); + }, + }; + $[0] = a; + $[1] = b; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 1, b: 2 }], +}; + +``` + +### Eval output +(kind: ok) {"test1":"[[ function params=0 ]]","test2":"[[ function params=0 ]]"} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-method-shorthand.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-method-shorthand.js new file mode 100644 index 000000000000..d25f77e5d5d2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-method-shorthand.js @@ -0,0 +1,15 @@ +function Component({a, b}) { + return { + test1() { + console.log(a); + }, + test2() { + console.log(b); + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 1, b: 2}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.expect.md new file mode 100644 index 000000000000..7b7e95953f71 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +function Component({a, b, c}) { + return { + test1() { + console.log(a); + }, + test2: () => { + console.log(b); + }, + test3: function () { + console.log(c); + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 1, b: 2, c: 3}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(t0) { + const $ = _c(8); + const { a, b, c } = t0; + let t1; + if ($[0] !== a || $[1] !== b || $[2] !== c) { + let t2; + if ($[4] !== b) { + t2 = () => { + console.log(b); + }; + $[4] = b; + $[5] = t2; + } else { + t2 = $[5]; + } + let t3; + if ($[6] !== c) { + t3 = function () { + console.log(c); + }; + $[6] = c; + $[7] = t3; + } else { + t3 = $[7]; + } + t1 = { + test1() { + console.log(a); + }, + test2: t2, + test3: t3, + }; + $[0] = a; + $[1] = b; + $[2] = c; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 1, b: 2, c: 3 }], +}; + +``` + +### Eval output +(kind: ok) {"test1":"[[ function params=0 ]]","test2":"[[ function params=0 ]]","test3":"[[ function params=0 ]]"} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.js new file mode 100644 index 000000000000..5abb5181f966 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.js @@ -0,0 +1,18 @@ +function Component({a, b, c}) { + return { + test1() { + console.log(a); + }, + test2: () => { + console.log(b); + }, + test3: function () { + console.log(c); + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 1, b: 2, c: 3}], +}; From 8608444b56e3b8c114b13c0c0fdc934d7bb46514 Mon Sep 17 00:00:00 2001 From: Bobadu Date: Fri, 27 Mar 2026 13:31:05 +0100 Subject: [PATCH 2/4] Extend AlignObjectMethodScopes to handle function expression properties (#36151) --- .../ReactiveScopes/AlignObjectMethodScopes.ts | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts index 317168f98685..0eff9c22ad10 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts @@ -17,33 +17,49 @@ import {eachInstructionValueOperand} from '../HIR/visitors'; import DisjointSet from '../Utils/DisjointSet'; /** - * Align scopes of object method values to that of their enclosing object expressions. - * To produce a well-formed JS program in Codegen, object methods and object expressions - * must be in the same ReactiveBlock as object method definitions must be inlined. + * Align scopes of object method values and function expression property values + * to that of their enclosing object expressions. This ensures that objects are + * memoized as a single unit regardless of whether properties use method shorthand + * syntax or function expression syntax. + * + * For ObjectMethod values, this is also required for correctness: object methods + * and object expressions must be in the same ReactiveBlock as object method + * definitions must be inlined. */ function findScopesToMerge(fn: HIRFunction): DisjointSet { const objectMethodDecls: Set = new Set(); + const fnExprDecls: Set = new Set(); const mergeScopesBuilder = new DisjointSet(); for (const [_, block] of fn.body.blocks) { for (const {lvalue, value} of block.instructions) { if (value.kind === 'ObjectMethod') { objectMethodDecls.add(lvalue.identifier); + } else if (value.kind === 'FunctionExpression') { + fnExprDecls.add(lvalue.identifier); } else if (value.kind === 'ObjectExpression') { for (const operand of eachInstructionValueOperand(value)) { - if (objectMethodDecls.has(operand.identifier)) { + if ( + objectMethodDecls.has(operand.identifier) || + fnExprDecls.has(operand.identifier) + ) { const operandScope = operand.identifier.scope; const lvalueScope = lvalue.identifier.scope; - CompilerError.invariant( - operandScope != null && lvalueScope != null, - { - reason: - 'Internal error: Expected all ObjectExpressions and ObjectMethods to have non-null scope.', - loc: GeneratedSource, - }, - ); + if (operandScope == null || lvalueScope == null) { + // FunctionExpressions used as object properties may not always + // have scopes (e.g. if they were pruned). ObjectMethods must + // always have scopes. + if (objectMethodDecls.has(operand.identifier)) { + CompilerError.invariant(false, { + reason: + 'Internal error: Expected all ObjectExpressions and ObjectMethods to have non-null scope.', + loc: GeneratedSource, + }); + } + continue; + } mergeScopesBuilder.union([operandScope, lvalueScope]); } } From 2164d20d916d4a805d153852b5a413ee5632002f Mon Sep 17 00:00:00 2001 From: Bobadu Date: Fri, 27 Mar 2026 13:55:28 +0100 Subject: [PATCH 3/4] Update fixture snapshots for consistent object memoization (#36151) --- ...memoization-object-fn-expression.expect.md | 39 +++++---------- ...sistent-memoization-object-mixed.expect.md | 30 +++-------- ...merge-consecutive-scopes-objects.expect.md | 50 +++++++++---------- .../object-shorthand-method-1.expect.md | 30 ++++------- 4 files changed, 54 insertions(+), 95 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.expect.md index 38508c2ad710..b6231e7670c6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-fn-expression.expect.md @@ -25,38 +25,25 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(t0) { - const $ = _c(7); + const $ = _c(3); const { a, b } = t0; let t1; - if ($[0] !== a) { - t1 = () => { - console.log(a); + if ($[0] !== a || $[1] !== b) { + t1 = { + test1: () => { + console.log(a); + }, + test2: function () { + console.log(b); + }, }; $[0] = a; - $[1] = t1; - } else { - t1 = $[1]; - } - let t2; - if ($[2] !== b) { - t2 = function () { - console.log(b); - }; - $[2] = b; - $[3] = t2; - } else { - t2 = $[3]; - } - let t3; - if ($[4] !== t1 || $[5] !== t2) { - t3 = { test1: t1, test2: t2 }; - $[4] = t1; - $[5] = t2; - $[6] = t3; + $[1] = b; + $[2] = t1; } else { - t3 = $[6]; + t1 = $[2]; } - return t3; + return t1; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.expect.md index 7b7e95953f71..2a845e65ba6b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-inconsistent-memoization-object-mixed.expect.md @@ -28,36 +28,20 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(t0) { - const $ = _c(8); + const $ = _c(4); const { a, b, c } = t0; let t1; if ($[0] !== a || $[1] !== b || $[2] !== c) { - let t2; - if ($[4] !== b) { - t2 = () => { - console.log(b); - }; - $[4] = b; - $[5] = t2; - } else { - t2 = $[5]; - } - let t3; - if ($[6] !== c) { - t3 = function () { - console.log(c); - }; - $[6] = c; - $[7] = t3; - } else { - t3 = $[7]; - } t1 = { test1() { console.log(a); }, - test2: t2, - test3: t3, + test2: () => { + console.log(b); + }, + test3: function () { + console.log(c); + }, }; $[0] = a; $[1] = b; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/merge-consecutive-scopes-objects.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/merge-consecutive-scopes-objects.expect.md index d31d366b1e0d..4ed0863da801 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/merge-consecutive-scopes-objects.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/merge-consecutive-scopes-objects.expect.md @@ -44,7 +44,7 @@ import { Stringify } from "shared-runtime"; // prevent scome scopes from merging, which concealed a bug with the merging logic. // By avoiding JSX we eliminate extraneous instructions and more accurately test the merging. function Component(props) { - const $ = _c(11); + const $ = _c(9); const [state, setState] = useState(0); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { @@ -63,40 +63,36 @@ function Component(props) { } let t2; if ($[3] !== state) { - t2 = () => setState(state + 1); + let t3; + if ($[5] === Symbol.for("react.memo_cache_sentinel")) { + t3 = ["increment"]; + $[5] = t3; + } else { + t3 = $[5]; + } + t2 = { + component: "button", + props: { + "data-testid": "button", + onClick: () => setState(state + 1), + children: t3, + }, + }; $[3] = state; $[4] = t2; } else { t2 = $[4]; } let t3; - if ($[5] === Symbol.for("react.memo_cache_sentinel")) { - t3 = ["increment"]; - $[5] = t3; - } else { - t3 = $[5]; - } - let t4; - if ($[6] !== t2) { - t4 = { - component: "button", - props: { "data-testid": "button", onClick: t2, children: t3 }, - }; - $[6] = t2; - $[7] = t4; - } else { - t4 = $[7]; - } - let t5; - if ($[8] !== t1 || $[9] !== t4) { - t5 = [t0, t1, t4]; - $[8] = t1; - $[9] = t4; - $[10] = t5; + if ($[6] !== t1 || $[7] !== t2) { + t3 = [t0, t1, t2]; + $[6] = t1; + $[7] = t2; + $[8] = t3; } else { - t5 = $[10]; + t3 = $[8]; } - return t5; + return t3; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-shorthand-method-1.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-shorthand-method-1.expect.md index 6b9ca0c2317b..7bbe85f48422 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-shorthand-method-1.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-shorthand-method-1.expect.md @@ -27,33 +27,25 @@ export const FIXTURE_ENTRYPOINT = { import { c as _c } from "react/compiler-runtime"; import { createHookWrapper } from "shared-runtime"; function useHook(t0) { - const $ = _c(5); + const $ = _c(3); const { a, b } = t0; let t1; - if ($[0] !== a) { - t1 = function () { - return [a]; - }; - $[0] = a; - $[1] = t1; - } else { - t1 = $[1]; - } - let t2; - if ($[2] !== b || $[3] !== t1) { - t2 = { - x: t1, + if ($[0] !== a || $[1] !== b) { + t1 = { + x: function () { + return [a]; + }, y() { return [b]; }, }; - $[2] = b; - $[3] = t1; - $[4] = t2; + $[0] = a; + $[1] = b; + $[2] = t1; } else { - t2 = $[4]; + t1 = $[2]; } - return t2; + return t1; } export const FIXTURE_ENTRYPOINT = { From 5b3fbd3e7ad36550d11a463f67a96dd6d3acecdd Mon Sep 17 00:00:00 2001 From: Bobadu Date: Fri, 27 Mar 2026 13:57:10 +0100 Subject: [PATCH 4/4] Fix lint: use block comment style in AlignObjectMethodScopes --- .../src/ReactiveScopes/AlignObjectMethodScopes.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts index 0eff9c22ad10..2058e0ee47f7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts @@ -48,9 +48,11 @@ function findScopesToMerge(fn: HIRFunction): DisjointSet { const lvalueScope = lvalue.identifier.scope; if (operandScope == null || lvalueScope == null) { - // FunctionExpressions used as object properties may not always - // have scopes (e.g. if they were pruned). ObjectMethods must - // always have scopes. + /* + * FunctionExpressions used as object properties may not always + * have scopes (e.g. if they were pruned). ObjectMethods must + * always have scopes. + */ if (objectMethodDecls.has(operand.identifier)) { CompilerError.invariant(false, { reason: