Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Comment on lines +1831 to +1840
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, the invariant was there to make sure that we don't rewrite a method invocation, which may read from its receiver. In the Math.* cases they are static functions and that isn't a problem, but i'm wondering if the wrong signature could break compilation w the change as implemented.

I think it would be something that a) has the receiver as readonly and b) returns a primitive. Can you try constructing a method like this in shared-runtime and type it (shared-runtime-type-provider) to check that it works correctly?

} 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': {
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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,
};
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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,
};

This file was deleted.

This file was deleted.

This file was deleted.

Loading
Loading