Compare commits

...

1 Commits

Author SHA1 Message Date
Lauren Tan
d545586fca [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.
2025-08-07 11:45:25 -04:00
12 changed files with 354 additions and 110 deletions

View File

@@ -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': {

View File

@@ -0,0 +1,69 @@
## Input
```javascript
import {PrimitiveBox} from 'shared-runtime';
function Component({value, realmax}) {
const box = new PrimitiveBox(value);
const maxValue = Math.max(box.get(), realmax);
// ^^^^^^^^^ should not be separated into static call
return <div>{maxValue}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: 42, realmax: 100}],
isComponent: true,
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime";
import { PrimitiveBox } from "shared-runtime";
function Component(t0) {
const $ = _c(6);
const { value, realmax } = t0;
let t1;
let t2;
let t3;
if ($[0] !== value) {
const box = new PrimitiveBox(value);
t1 = Math;
t2 = t1.max;
t3 = box.get();
$[0] = value;
$[1] = t1;
$[2] = t2;
$[3] = t3;
} else {
t1 = $[1];
t2 = $[2];
t3 = $[3];
}
const maxValue = t2(t3, realmax);
let t4;
if ($[4] !== maxValue) {
t4 = <div>{maxValue}</div>;
$[4] = maxValue;
$[5] = t4;
} else {
t4 = $[5];
}
return t4;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: 42, realmax: 100 }],
isComponent: true,
};
```
### Eval output
(kind: ok) <div>100</div>

View File

@@ -0,0 +1,14 @@
import {PrimitiveBox} from 'shared-runtime';
function Component({value, realmax}) {
const box = new PrimitiveBox(value);
const maxValue = Math.max(box.get(), realmax);
// ^^^^^^^^^ should not be separated into static call
return <div>{maxValue}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: 42, realmax: 100}],
isComponent: true,
};

View File

@@ -0,0 +1,85 @@
## Input
```javascript
function foo() {
return {
bar() {
return 3.14;
},
};
}
const YearsAndMonthsSince = () => {
const diff = foo();
const months = Math.floor(diff.bar());
return <>{months}</>;
};
export const FIXTURE_ENTRYPOINT = {
fn: YearsAndMonthsSince,
params: [],
isComponent: true,
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime";
function foo() {
const $ = _c(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = {
bar() {
return 3.14;
},
};
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}
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;
};
export const FIXTURE_ENTRYPOINT = {
fn: YearsAndMonthsSince,
params: [],
isComponent: true,
};
```
### Eval output
(kind: ok) 3

View File

@@ -0,0 +1,19 @@
function foo() {
return {
bar() {
return 3.14;
},
};
}
const YearsAndMonthsSince = () => {
const diff = foo();
const months = Math.floor(diff.bar());
return <>{months}</>;
};
export const FIXTURE_ENTRYPOINT = {
fn: YearsAndMonthsSince,
params: [],
isComponent: true,
};

View File

@@ -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 |
```

View File

@@ -1,5 +0,0 @@
const YearsAndMonthsSince = () => {
const diff = foo();
const months = Math.floor(diff.bar());
return <>{months}</>;
};

View File

@@ -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 |
```

View File

@@ -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

View File

@@ -196,6 +196,29 @@ export function makeSharedRuntimeTypeProvider({
],
},
},
PrimitiveBox: {
kind: 'object',
properties: {
get: {
kind: 'function',
positionalParams: [],
restParam: null,
calleeEffect: EffectEnum.Read,
returnType: {kind: 'type', name: 'Primitive'},
returnValueKind: ValueKindEnum.Primitive,
aliasing: {
receiver: '@receiver',
params: ['@value'],
rest: null,
returns: '@return',
temporaries: [],
effects: [
{kind: 'CreateFrom', from: '@value', into: '@return'},
],
},
},
},
},
},
};
} else if (moduleName === 'ReactCompilerTest') {

View File

@@ -421,4 +421,22 @@ export function typedMutate(x: any, v: any = null): void {
x.property = v;
}
type PrimitiveValue =
| number
| string
| boolean
| symbol
| null
| undefined
| bigint;
export class PrimitiveBox<T extends PrimitiveValue> {
value: T;
constructor(value: T) {
this.value = value;
}
get(): T {
return this.value;
}
}
export default typedLog;