Skip to content

Commit f6affef

Browse files
committed
[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 Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
1 parent 3958d5d commit f6affef

12 files changed

+355
-111
lines changed

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

Lines changed: 64 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,41 +1810,70 @@ function codegenInstructionValue(
18101810
case 'MethodCall': {
18111811
const isHook =
18121812
getHookKind(cx.env, instrValue.property.identifier) != null;
1813-
const memberExpr = codegenPlaceToExpression(cx, instrValue.property);
1814-
CompilerError.invariant(
1815-
t.isMemberExpression(memberExpr) ||
1816-
t.isOptionalMemberExpression(memberExpr),
1817-
{
1818-
reason:
1819-
'[Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. ' +
1820-
`Got a \`${memberExpr.type}\``,
1821-
description: null,
1822-
loc: memberExpr.loc ?? null,
1823-
suggestions: null,
1824-
},
1825-
);
1826-
CompilerError.invariant(
1827-
t.isNodesEquivalent(
1828-
memberExpr.object,
1829-
codegenPlaceToExpression(cx, instrValue.receiver),
1830-
),
1831-
{
1832-
reason:
1833-
'[Codegen] Internal error: Forget should always generate MethodCall::property ' +
1834-
'as a MemberExpression of MethodCall::receiver',
1835-
description: null,
1836-
loc: memberExpr.loc ?? null,
1837-
suggestions: null,
1838-
},
1839-
);
1840-
const args = instrValue.args.map(arg => codegenArgument(cx, arg));
1841-
value = createCallExpression(
1842-
cx.env,
1843-
memberExpr,
1844-
args,
1845-
instrValue.loc,
1846-
isHook,
1847-
);
1813+
/**
1814+
* We need to check if the property was memoized. If it has, we should reconstruct the
1815+
* MemberExpression.
1816+
*/
1817+
let memberExpr: t.Expression;
1818+
const tmp = cx.temp.get(instrValue.property.identifier.declarationId);
1819+
if (tmp != null && tmp.type === 'Identifier') {
1820+
/**
1821+
* We can't reconstruct the MemberExpression from just the identifier, so we work around
1822+
* this by allowing an Identifier here.
1823+
*/
1824+
memberExpr = tmp;
1825+
} else if (tmp != null) {
1826+
memberExpr = convertValueToExpression(tmp);
1827+
} else {
1828+
memberExpr = codegenPlaceToExpression(cx, instrValue.property);
1829+
}
1830+
1831+
// Reconstruct the MemberExpression if we previously saw an Identifier.
1832+
if (memberExpr.type === 'Identifier') {
1833+
const args = instrValue.args.map(arg => codegenArgument(cx, arg));
1834+
value = createCallExpression(
1835+
cx.env,
1836+
memberExpr,
1837+
args,
1838+
instrValue.loc,
1839+
isHook,
1840+
);
1841+
} else {
1842+
CompilerError.invariant(
1843+
t.isMemberExpression(memberExpr) ||
1844+
t.isOptionalMemberExpression(memberExpr),
1845+
{
1846+
reason:
1847+
'[Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. ' +
1848+
`Got a \`${memberExpr.type}\``,
1849+
description: null,
1850+
loc: memberExpr.loc ?? null,
1851+
suggestions: null,
1852+
},
1853+
);
1854+
CompilerError.invariant(
1855+
t.isNodesEquivalent(
1856+
memberExpr.object,
1857+
codegenPlaceToExpression(cx, instrValue.receiver),
1858+
),
1859+
{
1860+
reason:
1861+
'[Codegen] Internal error: Forget should always generate MethodCall::property ' +
1862+
'as a MemberExpression of MethodCall::receiver',
1863+
description: null,
1864+
loc: memberExpr.loc ?? null,
1865+
suggestions: null,
1866+
},
1867+
);
1868+
const args = instrValue.args.map(arg => codegenArgument(cx, arg));
1869+
value = createCallExpression(
1870+
cx.env,
1871+
memberExpr,
1872+
args,
1873+
instrValue.loc,
1874+
isHook,
1875+
);
1876+
}
18481877
break;
18491878
}
18501879
case 'NewExpression': {
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {PrimitiveBox} from 'shared-runtime';
6+
7+
function Component({value, realmax}) {
8+
const box = new PrimitiveBox(value);
9+
const maxValue = Math.max(box.get(), realmax);
10+
// ^^^^^^^^^ should not be separated into static call
11+
return <div>{maxValue}</div>;
12+
}
13+
14+
export const FIXTURE_ENTRYPOINT = {
15+
fn: Component,
16+
params: [{value: 42, realmax: 100}],
17+
isComponent: true,
18+
};
19+
20+
```
21+
22+
## Code
23+
24+
```javascript
25+
import { c as _c } from "react/compiler-runtime";
26+
import { PrimitiveBox } from "shared-runtime";
27+
28+
function Component(t0) {
29+
const $ = _c(6);
30+
const { value, realmax } = t0;
31+
let t1;
32+
let t2;
33+
let t3;
34+
if ($[0] !== value) {
35+
const box = new PrimitiveBox(value);
36+
t1 = Math;
37+
t2 = t1.max;
38+
t3 = box.get();
39+
$[0] = value;
40+
$[1] = t1;
41+
$[2] = t2;
42+
$[3] = t3;
43+
} else {
44+
t1 = $[1];
45+
t2 = $[2];
46+
t3 = $[3];
47+
}
48+
const maxValue = t2(t3, realmax);
49+
let t4;
50+
if ($[4] !== maxValue) {
51+
t4 = <div>{maxValue}</div>;
52+
$[4] = maxValue;
53+
$[5] = t4;
54+
} else {
55+
t4 = $[5];
56+
}
57+
return t4;
58+
}
59+
60+
export const FIXTURE_ENTRYPOINT = {
61+
fn: Component,
62+
params: [{ value: 42, realmax: 100 }],
63+
isComponent: true,
64+
};
65+
66+
```
67+
68+
### Eval output
69+
(kind: ok) <div>100</div>
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import {PrimitiveBox} from 'shared-runtime';
2+
3+
function Component({value, realmax}) {
4+
const box = new PrimitiveBox(value);
5+
const maxValue = Math.max(box.get(), realmax);
6+
// ^^^^^^^^^ should not be separated into static call
7+
return <div>{maxValue}</div>;
8+
}
9+
10+
export const FIXTURE_ENTRYPOINT = {
11+
fn: Component,
12+
params: [{value: 42, realmax: 100}],
13+
isComponent: true,
14+
};
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
2+
## Input
3+
4+
```javascript
5+
function foo() {
6+
return {
7+
bar() {
8+
return 3.14;
9+
},
10+
};
11+
}
12+
13+
const YearsAndMonthsSince = () => {
14+
const diff = foo();
15+
const months = Math.floor(diff.bar());
16+
return <>{months}</>;
17+
};
18+
19+
export const FIXTURE_ENTRYPOINT = {
20+
fn: YearsAndMonthsSince,
21+
params: [],
22+
isComponent: true,
23+
};
24+
25+
```
26+
27+
## Code
28+
29+
```javascript
30+
import { c as _c } from "react/compiler-runtime";
31+
function foo() {
32+
const $ = _c(1);
33+
let t0;
34+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
35+
t0 = {
36+
bar() {
37+
return 3.14;
38+
},
39+
};
40+
$[0] = t0;
41+
} else {
42+
t0 = $[0];
43+
}
44+
return t0;
45+
}
46+
47+
const YearsAndMonthsSince = () => {
48+
const $ = _c(4);
49+
let t0;
50+
let t1;
51+
let t2;
52+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
53+
const diff = foo();
54+
t0 = Math;
55+
t1 = t0.floor;
56+
t2 = diff.bar();
57+
$[0] = t0;
58+
$[1] = t1;
59+
$[2] = t2;
60+
} else {
61+
t0 = $[0];
62+
t1 = $[1];
63+
t2 = $[2];
64+
}
65+
const months = t1(t2);
66+
let t3;
67+
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
68+
t3 = <>{months}</>;
69+
$[3] = t3;
70+
} else {
71+
t3 = $[3];
72+
}
73+
return t3;
74+
};
75+
76+
export const FIXTURE_ENTRYPOINT = {
77+
fn: YearsAndMonthsSince,
78+
params: [],
79+
isComponent: true,
80+
};
81+
82+
```
83+
84+
### Eval output
85+
(kind: ok) 3
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
function foo() {
2+
return {
3+
bar() {
4+
return 3.14;
5+
},
6+
};
7+
}
8+
9+
const YearsAndMonthsSince = () => {
10+
const diff = foo();
11+
const months = Math.floor(diff.bar());
12+
return <>{months}</>;
13+
};
14+
15+
export const FIXTURE_ENTRYPOINT = {
16+
fn: YearsAndMonthsSince,
17+
params: [],
18+
isComponent: true,
19+
};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-codegen-methodcall.expect.md

Lines changed: 0 additions & 31 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-invariant-codegen-methodcall.js

Lines changed: 0 additions & 5 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md

Lines changed: 0 additions & 39 deletions
This file was deleted.

0 commit comments

Comments
 (0)