[compiler][bugfix] Fix hoisting of let declarations (#32724)
(Found when compiling Meta React code) Let variable declarations and reassignments are currently rewritten to `StoreLocal <varName>` instructions, which each translates to a new `const varName` declaration in codegen. ```js // Example input function useHook() { const getX = () => x; let x = CONSTANT1; if (cond) { x += CONSTANT2; } return <Stringify getX={getX} /> } // Compiled output, prior to this PR import { c as _c } from "react/compiler-runtime"; function useHook() { const $ = _c(1); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { const getX = () => x; let x = CONSTANT1; if (cond) { let x = x + CONSTANT2; x; } t0 = <Stringify getX={getX} />; $[0] = t0; } else { t0 = $[0]; } return t0; } ``` This also manifests as a babel internal error when replacing the original function declaration with the compiler output. The below compilation output fails with `Duplicate declaration "x" (This is an error on an internal node. Probably an internal error.)`. ```js // example input let x = CONSTANT1; if (cond) { x += CONSTANT2; x = CONSTANT3; } // current output let x = CONSTANT1; if (playheadDragState) { let x = x + CONSTANT2 x; let x = CONSTANT3; } ```
mofeiZ committed
Mar 24, 2025 at 14:30 UTC
254dc4d9f37eb512d4ee8bad6a0fae7ae491caef
13 files changed
+378
-27
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts
+80
-21
@@ -5,6 +5,7 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
+import {CompilerError} from '..';
9
import {
10
DeclarationId,
11
InstructionKind,
@@ -27,7 +28,17 @@ export function pruneHoistedContexts(fn: ReactiveFunction): void {
28
visitReactiveFunction(fn, new Visitor(), hoistedIdentifiers);
29
}
30
30
-type HoistedIdentifiers = Map<DeclarationId, InstructionKind>;
31
+const REWRITTEN_HOISTED_CONST: unique symbol = Symbol(
32
+ 'REWRITTEN_HOISTED_CONST',
33
+);
34
+const REWRITTEN_HOISTED_LET: unique symbol = Symbol('REWRITTEN_HOISTED_LET');
35
+
36
+type HoistedIdentifiers = Map<
37
+ DeclarationId,
38
+ | InstructionKind
39
+ | typeof REWRITTEN_HOISTED_CONST
40
+ | typeof REWRITTEN_HOISTED_LET
41
+>;
42
43
class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
44
override transformInstruction(
@@ -35,6 +46,10 @@ class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
46
state: HoistedIdentifiers,
47
): Transformed<ReactiveStatement> {
48
this.visitInstruction(instruction, state);
49
+
50
+ /**
51
+ * Remove hoisted declarations to preserve TDZ
52
+ */
53
if (
54
instruction.value.kind === 'DeclareContext' &&
55
instruction.value.lvalue.kind === 'HoistedConst'
@@ -68,31 +83,75 @@ class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
83
return {kind: 'remove'};
84
}
85
71
- if (
72
- instruction.value.kind === 'StoreContext' &&
73
- state.has(instruction.value.lvalue.place.identifier.declarationId)
74
- ) {
86
+ if (instruction.value.kind === 'StoreContext') {
87
const kind = state.get(
88
instruction.value.lvalue.place.identifier.declarationId,
77
- )!;
78
- return {
79
- kind: 'replace',
80
- value: {
81
- kind: 'instruction',
82
- instruction: {
83
- ...instruction,
89
+ );
90
+ if (kind != null) {
91
+ CompilerError.invariant(kind !== REWRITTEN_HOISTED_CONST, {
92
+ reason: 'Expected exactly one store to a hoisted const variable',
93
+ loc: instruction.loc,
94
+ });
95
+ if (
96
+ kind === InstructionKind.Const ||
97
+ kind === InstructionKind.Function
98
+ ) {
99
+ state.set(
100
+ instruction.value.lvalue.place.identifier.declarationId,
101
+ REWRITTEN_HOISTED_CONST,
102
+ );
103
+ return {
104
+ kind: 'replace',
105
value: {
85
- ...instruction.value,
86
- lvalue: {
87
- ...instruction.value.lvalue,
88
- kind,
106
+ kind: 'instruction',
107
+ instruction: {
108
+ ...instruction,
109
+ value: {
110
+ ...instruction.value,
111
+ lvalue: {
112
+ ...instruction.value.lvalue,
113
+ kind,
114
+ },
115
+ type: null,
116
+ kind: 'StoreLocal',
117
+ },
118
},
90
- type: null,
91
- kind: 'StoreLocal',
119
},
93
- },
94
- },
95
- };
120
+ };
121
+ } else if (kind !== REWRITTEN_HOISTED_LET) {
122
+ /**
123
+ * Context variables declared with let may have reassignments. Only
124
+ * insert a `DeclareContext` for the first encountered `StoreContext`
125
+ * instruction.
126
+ */
127
+ state.set(
128
+ instruction.value.lvalue.place.identifier.declarationId,
129
+ REWRITTEN_HOISTED_LET,
130
+ );
131
+ return {
132
+ kind: 'replace-many',
133
+ value: [
134
+ {
135
+ kind: 'instruction',
136
+ instruction: {
137
+ id: instruction.id,
138
+ lvalue: null,
139
+ value: {
140
+ kind: 'DeclareContext',
141
+ lvalue: {
142
+ kind: InstructionKind.Let,
143
+ place: {...instruction.value.lvalue.place},
144
+ },
145
+ loc: instruction.value.loc,
146
+ },
147
+ loc: instruction.loc,
148
+ },
149
+ },
150
+ {kind: 'instruction', instruction},
151
+ ],
152
+ };
153
+ }
154
+ }
155
}
156
157
return {kind: 'keep'};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-invalid-tdz-let.expect.md
new
+59
@@ -0,0 +1,59 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Foo() {
6
+ const getX = () => x;
7
+ console.log(getX());
8
+
9
+ let x = 4;
10
+ x += 5;
11
+
12
+ return <Stringify getX={getX} shouldInvokeFns={true} />;
13
+}
14
+
15
+export const FIXTURE_ENTRYPOINT = {
16
+ fn: Foo,
17
+ params: [],
18
+};
19
+
20
+```
21
+
22
+## Code
23
+
24
+```javascript
25
+import { c as _c } from "react/compiler-runtime";
26
+function Foo() {
27
+ const $ = _c(2);
28
+ let getX;
29
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
30
+ getX = () => x;
31
+ console.log(getX());
32
+
33
+ let x;
34
+ x = 4;
35
+ x = x + 5;
36
+ $[0] = getX;
37
+ } else {
38
+ getX = $[0];
39
+ }
40
+ x;
41
+ let t0;
42
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
43
+ t0 = <Stringify getX={getX} shouldInvokeFns={true} />;
44
+ $[1] = t0;
45
+ } else {
46
+ t0 = $[1];
47
+ }
48
+ return t0;
49
+}
50
+
51
+export const FIXTURE_ENTRYPOINT = {
52
+ fn: Foo,
53
+ params: [],
54
+};
55
+
56
+```
57
+
58
+### Eval output
59
+(kind: exception) Cannot access 'x' before initialization
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-invalid-tdz-let.js
new
+14
@@ -0,0 +1,14 @@
1
+function Foo() {
2
+ const getX = () => x;
3
+ console.log(getX());
4
+
5
+ let x = 4;
6
+ x += 5;
7
+
8
+ return <Stringify getX={getX} shouldInvokeFns={true} />;
9
+}
10
+
11
+export const FIXTURE_ENTRYPOINT = {
12
+ fn: Foo,
13
+ params: [],
14
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-nested-let-declaration-2.expect.md
+2
-1
@@ -36,7 +36,8 @@ function hoisting(cond) {
36
items.push(bar());
37
};
38
39
- let bar = _temp;
39
+ let bar;
40
+ bar = _temp;
41
foo();
42
}
43
$[0] = cond;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-nested-let-declaration.expect.md
+4
-2
@@ -41,9 +41,11 @@ function hoisting() {
41
return result;
42
};
43
44
- let foo = () => bar + baz;
44
+ let foo;
45
+ foo = () => bar + baz;
46
46
- let bar = 3;
47
+ let bar;
48
+ bar = 3;
49
const baz = 2;
50
t0 = qux();
51
$[0] = t0;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-reassigned-let-declaration.expect.md
new
+67
@@ -0,0 +1,67 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import {CONST_NUMBER0, CONST_NUMBER1, Stringify} from 'shared-runtime';
6
+
7
+function useHook({cond}) {
8
+ 'use memo';
9
+ const getX = () => x;
10
+
11
+ let x = CONST_NUMBER0;
12
+ if (cond) {
13
+ x += CONST_NUMBER1;
14
+ }
15
+ return <Stringify getX={getX} shouldInvokeFns={true} />;
16
+}
17
+
18
+export const FIXTURE_ENTRYPOINT = {
19
+ fn: useHook,
20
+ params: [{cond: true}],
21
+ sequentialRenders: [{cond: true}, {cond: true}, {cond: false}],
22
+};
23
+
24
+```
25
+
26
+## Code
27
+
28
+```javascript
29
+import { c as _c } from "react/compiler-runtime";
30
+import { CONST_NUMBER0, CONST_NUMBER1, Stringify } from "shared-runtime";
31
+
32
+function useHook(t0) {
33
+ "use memo";
34
+ const $ = _c(2);
35
+ const { cond } = t0;
36
+ let t1;
37
+ if ($[0] !== cond) {
38
+ const getX = () => x;
39
+
40
+ let x;
41
+ x = CONST_NUMBER0;
42
+ if (cond) {
43
+ x = x + CONST_NUMBER1;
44
+ x;
45
+ }
46
+
47
+ t1 = <Stringify getX={getX} shouldInvokeFns={true} />;
48
+ $[0] = cond;
49
+ $[1] = t1;
50
+ } else {
51
+ t1 = $[1];
52
+ }
53
+ return t1;
54
+}
55
+
56
+export const FIXTURE_ENTRYPOINT = {
57
+ fn: useHook,
58
+ params: [{ cond: true }],
59
+ sequentialRenders: [{ cond: true }, { cond: true }, { cond: false }],
60
+};
61
+
62
+```
63
+
64
+### Eval output
65
+(kind: ok) <div>{"getX":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
66
+<div>{"getX":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
67
+<div>{"getX":{"kind":"Function","result":0},"shouldInvokeFns":true}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-reassigned-let-declaration.js
new
+18
@@ -0,0 +1,18 @@
1
+import {CONST_NUMBER0, CONST_NUMBER1, Stringify} from 'shared-runtime';
2
+
3
+function useHook({cond}) {
4
+ 'use memo';
5
+ const getX = () => x;
6
+
7
+ let x = CONST_NUMBER0;
8
+ if (cond) {
9
+ x += CONST_NUMBER1;
10
+ }
11
+ return <Stringify getX={getX} shouldInvokeFns={true} />;
12
+}
13
+
14
+export const FIXTURE_ENTRYPOINT = {
15
+ fn: useHook,
16
+ params: [{cond: true}],
17
+ sequentialRenders: [{cond: true}, {cond: true}, {cond: false}],
18
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-reassigned-twice-let-declaration.expect.md
new
+69
@@ -0,0 +1,69 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import {CONST_NUMBER0, CONST_NUMBER1, Stringify} from 'shared-runtime';
6
+
7
+function useHook({cond}) {
8
+ 'use memo';
9
+ const getX = () => x;
10
+
11
+ let x = CONST_NUMBER0;
12
+ if (cond) {
13
+ x += CONST_NUMBER1;
14
+ x = Math.min(x, 100);
15
+ }
16
+ return <Stringify getX={getX} shouldInvokeFns={true} />;
17
+}
18
+
19
+export const FIXTURE_ENTRYPOINT = {
20
+ fn: useHook,
21
+ params: [{cond: true}],
22
+ sequentialRenders: [{cond: true}, {cond: true}, {cond: false}],
23
+};
24
+
25
+```
26
+
27
+## Code
28
+
29
+```javascript
30
+import { c as _c } from "react/compiler-runtime";
31
+import { CONST_NUMBER0, CONST_NUMBER1, Stringify } from "shared-runtime";
32
+
33
+function useHook(t0) {
34
+ "use memo";
35
+ const $ = _c(2);
36
+ const { cond } = t0;
37
+ let t1;
38
+ if ($[0] !== cond) {
39
+ const getX = () => x;
40
+
41
+ let x;
42
+ x = CONST_NUMBER0;
43
+ if (cond) {
44
+ x = x + CONST_NUMBER1;
45
+ x;
46
+ x = Math.min(x, 100);
47
+ }
48
+
49
+ t1 = <Stringify getX={getX} shouldInvokeFns={true} />;
50
+ $[0] = cond;
51
+ $[1] = t1;
52
+ } else {
53
+ t1 = $[1];
54
+ }
55
+ return t1;
56
+}
57
+
58
+export const FIXTURE_ENTRYPOINT = {
59
+ fn: useHook,
60
+ params: [{ cond: true }],
61
+ sequentialRenders: [{ cond: true }, { cond: true }, { cond: false }],
62
+};
63
+
64
+```
65
+
66
+### Eval output
67
+(kind: ok) <div>{"getX":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
68
+<div>{"getX":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
69
+<div>{"getX":{"kind":"Function","result":0},"shouldInvokeFns":true}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-reassigned-twice-let-declaration.js
new
+19
@@ -0,0 +1,19 @@
1
+import {CONST_NUMBER0, CONST_NUMBER1, Stringify} from 'shared-runtime';
2
+
3
+function useHook({cond}) {
4
+ 'use memo';
5
+ const getX = () => x;
6
+
7
+ let x = CONST_NUMBER0;
8
+ if (cond) {
9
+ x += CONST_NUMBER1;
10
+ x = Math.min(x, 100);
11
+ }
12
+ return <Stringify getX={getX} shouldInvokeFns={true} />;
13
+}
14
+
15
+export const FIXTURE_ENTRYPOINT = {
16
+ fn: useHook,
17
+ params: [{cond: true}],
18
+ sequentialRenders: [{cond: true}, {cond: true}, {cond: false}],
19
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-simple-let-declaration.expect.md
+4
-2
@@ -29,8 +29,10 @@ function hoisting() {
29
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
30
foo = () => bar + baz;
31
32
- let bar = 3;
33
- let baz = 2;
32
+ let bar;
33
+ bar = 3;
34
+ let baz;
35
+ baz = 2;
36
$[0] = foo;
37
} else {
38
foo = $[0];
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mutate-captured-arg-separately.expect.md
+2
-1
@@ -33,7 +33,8 @@ function component(a) {
33
m(x);
34
};
35
36
- let x = { a };
36
+ let x;
37
+ x = { a };
38
m(x);
39
$[0] = a;
40
$[1] = y;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/recursive-function-expression.expect.md
+29
@@ -2,6 +2,17 @@
2
## Input
3
4
```javascript
5
+function Component1() {
6
+ const x = callback(10);
7
+ function callback(x) {
8
+ if (x == 0) {
9
+ return null;
10
+ }
11
+ return callback(x - 1);
12
+ }
13
+ return x;
14
+}
15
+
16
function Component() {
17
function callback(x) {
18
if (x == 0) {
@@ -23,6 +34,24 @@ export const FIXTURE_ENTRYPOINT = {
34
35
```javascript
36
import { c as _c } from "react/compiler-runtime";
37
+function Component1() {
38
+ const $ = _c(1);
39
+ let x;
40
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
41
+ x = callback(10);
42
+ function callback(x_0) {
43
+ if (x_0 == 0) {
44
+ return null;
45
+ }
46
+ return callback(x_0 - 1);
47
+ }
48
+ $[0] = x;
49
+ } else {
50
+ x = $[0];
51
+ }
52
+ return x;
53
+}
54
+
55
function Component() {
56
const $ = _c(1);
57
let t0;
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/recursive-function-expression.js
+11
@@ -1,3 +1,14 @@
1
+function Component1() {
2
+ const x = callback(10);
3
+ function callback(x) {
4
+ if (x == 0) {
5
+ return null;
6
+ }
7
+ return callback(x - 1);
8
+ }
9
+ return x;
10
+}
11
+
12
function Component() {
13
function callback(x) {
14
if (x == 0) {