Skip to content

Commit

Permalink
[compiler] Lower JSXMemberExpression with LoadLocal
Browse files Browse the repository at this point in the history
`JSXMemberExpression` is currently the only instruction (that I know of) that directly references identifier lvalues without a corresponding `LoadLocal`.

This has some side effects:
- deadcode elimination and constant propagation now reach JSXMemberExpressions
- we can delete `LoweredFunction.dependencies` without dangling references (previously, the only reference to JSXMemberExpression objects in HIR was in function dependencies)
- JSXMemberExpression now is consistent with all other instructions (e.g. has a rvalue-producing LoadLocal)

'
  • Loading branch information
mofeiZ committed Oct 24, 2024
1 parent 754d933 commit 857947f
Show file tree
Hide file tree
Showing 14 changed files with 299 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3186,7 +3186,13 @@ function lowerJsxMemberExpression(
loc: object.node.loc ?? null,
suggestions: null,
});
objectPlace = lowerIdentifier(builder, object);

const kind = getLoadKind(builder, object);
objectPlace = lowerValueToTemporary(builder, {
kind: kind,
place: lowerIdentifier(builder, object),
loc: exprPath.node.loc ?? GeneratedSource,
});
}
const property = exprPath.get('property').node.name;
return lowerValueToTemporary(builder, {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@

## Input

```javascript
import {Throw} from 'shared-runtime';

/**
* Note: this is disabled in the evaluator due to different devmode errors.
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok) <invalidtag val="[object Object]"></invalidtag>
* logs: ['Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag']
*
* Forget:
* (kind: ok) <invalidtag val="[object Object]"></invalidtag>
* logs: [
* 'Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag',
* 'Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','invalidTag',
* ]
*/
function useFoo() {
const invalidTag = Throw;
/**
* Need to be careful to not parse `invalidTag` as a localVar (i.e. render
* Throw). Note that the jsx transform turns this into a string tag:
* `jsx("invalidTag"...
*/
return <invalidTag val={{val: 2}} />;
}
export const FIXTURE_ENTRYPOINT = {
fn: useFoo,
params: [],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import { Throw } from "shared-runtime";

/**
* Note: this is disabled in the evaluator due to different devmode errors.
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok) <invalidtag val="[object Object]"></invalidtag>
* logs: ['Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag']
*
* Forget:
* (kind: ok) <invalidtag val="[object Object]"></invalidtag>
* logs: [
* 'Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag',
* 'Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','invalidTag',
* ]
*/
function useFoo() {
const $ = _c(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = <invalidTag val={{ val: 2 }} />;
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}

export const FIXTURE_ENTRYPOINT = {
fn: useFoo,
params: [],
};

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import {Throw} from 'shared-runtime';

/**
* Note: this is disabled in the evaluator due to different devmode errors.
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok) <invalidtag val="[object Object]"></invalidtag>
* logs: ['Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag']
*
* Forget:
* (kind: ok) <invalidtag val="[object Object]"></invalidtag>
* logs: [
* 'Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s','invalidTag',
* 'Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','invalidTag',
* ]
*/
function useFoo() {
const invalidTag = Throw;
/**
* Need to be careful to not parse `invalidTag` as a localVar (i.e. render
* Throw). Note that the jsx transform turns this into a string tag:
* `jsx("invalidTag"...
*/
return <invalidTag val={{val: 2}} />;
}
export const FIXTURE_ENTRYPOINT = {
fn: useFoo,
params: [],
};
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,10 @@ import * as SharedRuntime from "shared-runtime";
function useFoo(t0) {
const $ = _c(1);
const { cond } = t0;
const MyLocal = SharedRuntime;
if (cond) {
let t1;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t1 = <MyLocal.Text value={4} />;
t1 = <SharedRuntime.Text value={4} />;
$[0] = t1;
} else {
t1 = $[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ import { c as _c } from "react/compiler-runtime";
import * as SharedRuntime from "shared-runtime";
function useFoo() {
const $ = _c(1);
const MyLocal = SharedRuntime;
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = <MyLocal.Text value={4} />;
t0 = <SharedRuntime.Text value={4} />;
$[0] = t0;
} else {
t0 = $[0];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@

## Input

```javascript
import * as SharedRuntime from 'shared-runtime';
import {invoke} from 'shared-runtime';
function useComponentFactory({name}) {
const localVar = SharedRuntime;
const cb = () => <localVar.Stringify>hello world {name}</localVar.Stringify>;
return invoke(cb);
}

export const FIXTURE_ENTRYPOINT = {
fn: useComponentFactory,
params: [{name: 'sathya'}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import * as SharedRuntime from "shared-runtime";
import { invoke } from "shared-runtime";
function useComponentFactory(t0) {
const $ = _c(4);
const { name } = t0;
let t1;
if ($[0] !== name) {
t1 = () => (
<SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>
);
$[0] = name;
$[1] = t1;
} else {
t1 = $[1];
}
const cb = t1;
let t2;
if ($[2] !== cb) {
t2 = invoke(cb);
$[2] = cb;
$[3] = t2;
} else {
t2 = $[3];
}
return t2;
}

export const FIXTURE_ENTRYPOINT = {
fn: useComponentFactory,
params: [{ name: "sathya" }],
};

```
### Eval output
(kind: ok) <div>{"children":["hello world ","sathya"]}</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as SharedRuntime from 'shared-runtime';
import {invoke} from 'shared-runtime';
function useComponentFactory({name}) {
const localVar = SharedRuntime;
const cb = () => <localVar.Stringify>hello world {name}</localVar.Stringify>;
return invoke(cb);
}

export const FIXTURE_ENTRYPOINT = {
fn: useComponentFactory,
params: [{name: 'sathya'}],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@

## Input

```javascript
import * as SharedRuntime from 'shared-runtime';
function Component({name}) {
const localVar = SharedRuntime;
return <localVar.Stringify>hello world {name}</localVar.Stringify>;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{name: 'sathya'}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import * as SharedRuntime from "shared-runtime";
function Component(t0) {
const $ = _c(2);
const { name } = t0;
let t1;
if ($[0] !== name) {
t1 = <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>;
$[0] = name;
$[1] = t1;
} else {
t1 = $[1];
}
return t1;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ name: "sathya" }],
};

```
### Eval output
(kind: ok) <div>{"children":["hello world ","sathya"]}</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as SharedRuntime from 'shared-runtime';
function Component({name}) {
const localVar = SharedRuntime;
return <localVar.Stringify>hello world {name}</localVar.Stringify>;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{name: 'sathya'}],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@

## Input

```javascript
import * as SharedRuntime from 'shared-runtime';
function Component({name}) {
return <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{name: 'sathya'}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import * as SharedRuntime from "shared-runtime";
function Component(t0) {
const $ = _c(2);
const { name } = t0;
let t1;
if ($[0] !== name) {
t1 = <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>;
$[0] = name;
$[1] = t1;
} else {
t1 = $[1];
}
return t1;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ name: "sathya" }],
};

```
### Eval output
(kind: ok) <div>{"children":["hello world ","sathya"]}</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as SharedRuntime from 'shared-runtime';
function Component({name}) {
return <SharedRuntime.Stringify>hello world {name}</SharedRuntime.Stringify>;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{name: 'sathya'}],
};
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ import { c as _c } from "react/compiler-runtime";
import * as SharedRuntime from "shared-runtime";
function useFoo() {
const $ = _c(1);
const MyLocal = SharedRuntime;
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
const callback = () => <MyLocal.Text value={4} />;
const callback = () => <SharedRuntime.Text value={4} />;

t0 = callback();
$[0] = t0;
Expand Down
3 changes: 3 additions & 0 deletions compiler/packages/snap/src/SproutTodoFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,9 @@ const skipFilter = new Set([
'rules-of-hooks/rules-of-hooks-93dc5d5e538a',
'rules-of-hooks/rules-of-hooks-69521d94fa03',

// false positives
'invalid-jsx-lowercase-localvar',

// bugs
'fbt/bug-fbt-plural-multiple-function-calls',
'fbt/bug-fbt-plural-multiple-mixed-call-tag',
Expand Down
3 changes: 3 additions & 0 deletions compiler/packages/snap/src/sprout/shared-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@ export function Stringify(props: any): React.ReactElement {
toJSON(props, props?.shouldInvokeFns),
);
}
export function Throw() {
throw new Error();
}

export function ValidateMemoization({
inputs,
Expand Down

0 comments on commit 857947f

Please sign in to comment.