Skip to content

Commit fe81314

Browse files
authored
[compiler] Check TSAsExpression and TSNonNullExpression reorderability (facebook#33788)
## Summary The `TSAsExpression` and `TSNonNullExpression` nodes are supported by `lowerExpression()` but `isReorderableExpression()` does not check if they can be reordered. This PR updates `isReorderableExpression()` to handle these two node types by adding cases that fall through to the existing `TypeCastExpression` case. We ran `react-compiler-healthcheck` at scale on several of our repos and found dozens of `` (BuildHIR::node.lowerReorderableExpression) Expression type `TSAsExpression` cannot be safely reordered`` errors and a handful for `TSNonNullExpression`. ## How did you test this change? In this case I added two fixture tests
1 parent 2f0e7e5 commit fe81314

File tree

5 files changed

+150
-0
lines changed

5 files changed

+150
-0
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3001,6 +3001,8 @@ function isReorderableExpression(
30013001
}
30023002
}
30033003
}
3004+
case 'TSAsExpression':
3005+
case 'TSNonNullExpression':
30043006
case 'TypeCastExpression': {
30053007
return isReorderableExpression(
30063008
builder,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
2+
## Input
3+
4+
```javascript
5+
type Status = 'pending' | 'success' | 'error';
6+
7+
const StatusIndicator = ({status}: {status: Status}) => {
8+
return <div className={`status-${status}`}>Status: {status}</div>;
9+
};
10+
11+
const Component = ({status = 'pending' as Status}) => {
12+
return <StatusIndicator status={status} />;
13+
};
14+
15+
export const FIXTURE_ENTRYPOINT = {
16+
fn: Component,
17+
params: [{status: 'success'}],
18+
};
19+
20+
```
21+
22+
## Code
23+
24+
```javascript
25+
import { c as _c } from "react/compiler-runtime";
26+
type Status = "pending" | "success" | "error";
27+
28+
const StatusIndicator = (t0) => {
29+
const $ = _c(3);
30+
const { status } = t0;
31+
const t1 = `status-${status}`;
32+
let t2;
33+
if ($[0] !== status || $[1] !== t1) {
34+
t2 = <div className={t1}>Status: {status}</div>;
35+
$[0] = status;
36+
$[1] = t1;
37+
$[2] = t2;
38+
} else {
39+
t2 = $[2];
40+
}
41+
return t2;
42+
};
43+
44+
const Component = (t0) => {
45+
const $ = _c(2);
46+
const { status: t1 } = t0;
47+
const status = t1 === undefined ? ("pending" as Status) : t1;
48+
let t2;
49+
if ($[0] !== status) {
50+
t2 = <StatusIndicator status={status} />;
51+
$[0] = status;
52+
$[1] = t2;
53+
} else {
54+
t2 = $[1];
55+
}
56+
return t2;
57+
};
58+
59+
export const FIXTURE_ENTRYPOINT = {
60+
fn: Component,
61+
params: [{ status: "success" }],
62+
};
63+
64+
```
65+
66+
### Eval output
67+
(kind: ok) <div class="status-success">Status: success</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
type Status = 'pending' | 'success' | 'error';
2+
3+
const StatusIndicator = ({status}: {status: Status}) => {
4+
return <div className={`status-${status}`}>Status: {status}</div>;
5+
};
6+
7+
const Component = ({status = 'pending' as Status}) => {
8+
return <StatusIndicator status={status} />;
9+
};
10+
11+
export const FIXTURE_ENTRYPOINT = {
12+
fn: Component,
13+
params: [{status: 'success'}],
14+
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
2+
## Input
3+
4+
```javascript
5+
const THEME_MAP: ReadonlyMap<string, string> = new Map([
6+
['default', 'light'],
7+
['dark', 'dark'],
8+
]);
9+
10+
export const Component = ({theme = THEME_MAP.get('default')!}) => {
11+
return <div className={`theme-${theme}`}>User preferences</div>;
12+
};
13+
14+
export const FIXTURE_ENTRYPOINT = {
15+
fn: Component,
16+
params: [{status: 'success'}],
17+
};
18+
19+
```
20+
21+
## Code
22+
23+
```javascript
24+
import { c as _c } from "react/compiler-runtime";
25+
const THEME_MAP: ReadonlyMap<string, string> = new Map([
26+
["default", "light"],
27+
["dark", "dark"],
28+
]);
29+
30+
export const Component = (t0) => {
31+
const $ = _c(2);
32+
const { theme: t1 } = t0;
33+
const theme = t1 === undefined ? THEME_MAP.get("default") : t1;
34+
const t2 = `theme-${theme}`;
35+
let t3;
36+
if ($[0] !== t2) {
37+
t3 = <div className={t2}>User preferences</div>;
38+
$[0] = t2;
39+
$[1] = t3;
40+
} else {
41+
t3 = $[1];
42+
}
43+
return t3;
44+
};
45+
46+
export const FIXTURE_ENTRYPOINT = {
47+
fn: Component,
48+
params: [{ status: "success" }],
49+
};
50+
51+
```
52+
53+
### Eval output
54+
(kind: ok) <div class="theme-light">User preferences</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
const THEME_MAP: ReadonlyMap<string, string> = new Map([
2+
['default', 'light'],
3+
['dark', 'dark'],
4+
]);
5+
6+
export const Component = ({theme = THEME_MAP.get('default')!}) => {
7+
return <div className={`theme-${theme}`}>User preferences</div>;
8+
};
9+
10+
export const FIXTURE_ENTRYPOINT = {
11+
fn: Component,
12+
params: [{status: 'success'}],
13+
};

0 commit comments

Comments
 (0)