From 124ea85d6f8f7b56e4e5bdb90baba1f2d89eae8a Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Mon, 4 Aug 2025 11:55:34 -0400 Subject: [PATCH] [compiler] Fix codegen for nested method calls with memoized properties When processing nested method calls like `Math.floor(diff.bar())`, the compiler would trigger an invariant that `MethodCall::property must be a MemberExpression but got an Identifier`. The issue occurred when the property (e.g., Math.floor) was memoized in a reactive scope and promoted to a named identifier. Later during codegen, retrieving this memoized temporary would return just an Identifier instead of the expected MemberExpression. The fix handles this case by checking if the property has been memoized as an Identifier and using it directly for the call expression, rather than requiring it to be a MemberExpression. This fixes two test cases that were previously failing: - error.bug-invariant-codegen-methodcall - error.todo-nested-method-calls-lower-property-load-into-temporary --- .../ReactiveScopes/CodegenReactiveFunction.ts | 99 ++++++++++++------- .../compiler/codegen-methodcall.expect.md | 49 +++++++++ ...en-methodcall.js => codegen-methodcall.js} | 0 ...bug-invariant-codegen-methodcall.expect.md | 31 ------ ...wer-property-load-into-temporary.expect.md | 39 -------- ...wer-property-load-into-temporary.expect.md | 62 ++++++++++++ ...lls-lower-property-load-into-temporary.js} | 0 7 files changed, 175 insertions(+), 105 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-methodcall.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{error.bug-invariant-codegen-methodcall.js => codegen-methodcall.js} (100%) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-codegen-methodcall.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nested-method-calls-lower-property-load-into-temporary.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{error.todo-nested-method-calls-lower-property-load-into-temporary.js => nested-method-calls-lower-property-load-into-temporary.js} (100%) diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index f7da522954..ee91d8d568 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1810,41 +1810,70 @@ function codegenInstructionValue( case 'MethodCall': { const isHook = getHookKind(cx.env, instrValue.property.identifier) != null; - const memberExpr = codegenPlaceToExpression(cx, instrValue.property); - CompilerError.invariant( - t.isMemberExpression(memberExpr) || - t.isOptionalMemberExpression(memberExpr), - { - reason: - '[Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. ' + - `Got a \`${memberExpr.type}\``, - description: null, - loc: memberExpr.loc ?? null, - suggestions: null, - }, - ); - CompilerError.invariant( - t.isNodesEquivalent( - memberExpr.object, - codegenPlaceToExpression(cx, instrValue.receiver), - ), - { - reason: - '[Codegen] Internal error: Forget should always generate MethodCall::property ' + - 'as a MemberExpression of MethodCall::receiver', - description: null, - loc: memberExpr.loc ?? null, - suggestions: null, - }, - ); - const args = instrValue.args.map(arg => codegenArgument(cx, arg)); - value = createCallExpression( - cx.env, - memberExpr, - args, - instrValue.loc, - isHook, - ); + /** + * We need to check if the property was memoized. If it has, we should reconstruct the + * MemberExpression. + **/ + let memberExpr: t.Expression; + const tmp = cx.temp.get(instrValue.property.identifier.declarationId); + if (tmp != null && tmp.type === 'Identifier') { + /** + * We can't reconstruct the MemberExpression from just the identifier, so we work around + * this by allowing an Identifier here. + */ + memberExpr = tmp; + } else if (tmp != null) { + memberExpr = convertValueToExpression(tmp); + } else { + memberExpr = codegenPlaceToExpression(cx, instrValue.property); + } + + // Reconstruct the MemberExpression if we previously saw an Identifier. + if (memberExpr.type === 'Identifier') { + const args = instrValue.args.map(arg => codegenArgument(cx, arg)); + value = createCallExpression( + cx.env, + memberExpr, + args, + instrValue.loc, + isHook, + ); + } else { + CompilerError.invariant( + t.isMemberExpression(memberExpr) || + t.isOptionalMemberExpression(memberExpr), + { + reason: + '[Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. ' + + `Got a \`${memberExpr.type}\``, + description: null, + loc: memberExpr.loc ?? null, + suggestions: null, + }, + ); + CompilerError.invariant( + t.isNodesEquivalent( + memberExpr.object, + codegenPlaceToExpression(cx, instrValue.receiver), + ), + { + reason: + '[Codegen] Internal error: Forget should always generate MethodCall::property ' + + 'as a MemberExpression of MethodCall::receiver', + description: null, + loc: memberExpr.loc ?? null, + suggestions: null, + }, + ); + const args = instrValue.args.map(arg => codegenArgument(cx, arg)); + value = createCallExpression( + cx.env, + memberExpr, + args, + instrValue.loc, + isHook, + ); + } break; } case 'NewExpression': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-methodcall.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-methodcall.expect.md new file mode 100644 index 0000000000..84333c086a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-methodcall.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +const YearsAndMonthsSince = () => { + const diff = foo(); + const months = Math.floor(diff.bar()); + return <>{months}; +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +const YearsAndMonthsSince = () => { + const $ = _c(4); + let t0; + let t1; + let t2; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const diff = foo(); + t0 = Math; + t1 = t0.floor; + t2 = diff.bar(); + $[0] = t0; + $[1] = t1; + $[2] = t2; + } else { + t0 = $[0]; + t1 = $[1]; + t2 = $[2]; + } + const months = t1(t2); + let t3; + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + t3 = <>{months}; + $[3] = t3; + } else { + t3 = $[3]; + } + return t3; +}; + +``` + +### 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.bug-invariant-codegen-methodcall.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-methodcall.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-codegen-methodcall.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-methodcall.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-codegen-methodcall.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-codegen-methodcall.expect.md deleted file mode 100644 index 4ea831de87..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-codegen-methodcall.expect.md +++ /dev/null @@ -1,31 +0,0 @@ - -## Input - -```javascript -const YearsAndMonthsSince = () => { - const diff = foo(); - const months = Math.floor(diff.bar()); - return <>{months}; -}; - -``` - - -## Error - -``` -Found 1 error: - -Invariant: [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier` - -error.bug-invariant-codegen-methodcall.ts:3:17 - 1 | const YearsAndMonthsSince = () => { - 2 | const diff = foo(); -> 3 | const months = Math.floor(diff.bar()); - | ^^^^^^^^^^ [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier` - 4 | return <>{months}; - 5 | }; - 6 | -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md deleted file mode 100644 index 67d6c4f4e0..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md +++ /dev/null @@ -1,39 +0,0 @@ - -## Input - -```javascript -import {makeArray} from 'shared-runtime'; - -const other = [0, 1]; -function Component({}) { - const items = makeArray(0, 1, 2, null, 4, false, 6); - const max = Math.max(2, items.push(5), ...other); - return max; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{}], -}; - -``` - - -## Error - -``` -Found 1 error: - -Invariant: [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier` - -error.todo-nested-method-calls-lower-property-load-into-temporary.ts:6:14 - 4 | function Component({}) { - 5 | const items = makeArray(0, 1, 2, null, 4, false, 6); -> 6 | const max = Math.max(2, items.push(5), ...other); - | ^^^^^^^^ [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier` - 7 | return max; - 8 | } - 9 | -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nested-method-calls-lower-property-load-into-temporary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nested-method-calls-lower-property-load-into-temporary.expect.md new file mode 100644 index 0000000000..638e993014 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nested-method-calls-lower-property-load-into-temporary.expect.md @@ -0,0 +1,62 @@ + +## Input + +```javascript +import {makeArray} from 'shared-runtime'; + +const other = [0, 1]; +function Component({}) { + const items = makeArray(0, 1, 2, null, 4, false, 6); + const max = Math.max(2, items.push(5), ...other); + return max; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray } from "shared-runtime"; + +const other = [0, 1]; +function Component(t0) { + const $ = _c(4); + let t1; + let t2; + let t3; + let t4; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const items = makeArray(0, 1, 2, null, 4, false, 6); + t1 = Math; + t2 = t1.max; + t3 = 2; + t4 = items.push(5); + $[0] = t1; + $[1] = t2; + $[2] = t3; + $[3] = t4; + } else { + t1 = $[0]; + t2 = $[1]; + t3 = $[2]; + t4 = $[3]; + } + const max = t2(t3, t4, ...other); + return max; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok) 8 \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nested-method-calls-lower-property-load-into-temporary.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nested-method-calls-lower-property-load-into-temporary.js