@samitouri / QOS-React / commits / a601d1da36

[compiler] Allow lets to be hoisted

ghstack-source-id: 02f4698bd98705a855deb0d4bc30b9829afdddc0 Pull Request resolved: https://github.com/facebook/react/pull/30674

Mike Vitousek committed Aug 13, 2024 at 16:35 UTC a601d1da3647ed9a20d9f83b87c8019492f24215
19 files changed +381 -51
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
+13 -3
@@ -439,8 +439,11 @@ function lowerStatement(
439 loc: id.parentPath.node.loc ?? GeneratedSource,
440 });
441 continue;
442 - } else if (binding.kind !== 'const' && binding.kind !== 'var') {
443 - // Avoid double errors on var declarations, which we do not plan to support anyways
442 + } else if (
443 + binding.kind !== 'const' &&
444 + binding.kind !== 'var' &&
445 + binding.kind !== 'let'
446 + ) {
447 builder.errors.push({
448 severity: ErrorSeverity.Todo,
449 reason: 'Handle non-const declarations for hoisting',
@@ -463,10 +466,17 @@ function lowerStatement(
466 reactive: false,
467 loc: id.node.loc ?? GeneratedSource,
468 };
469 + const kind =
470 + // Avoid double errors on var declarations, which we do not plan to support anyways
471 + binding.kind === 'const' || binding.kind === 'var'
472 + ? InstructionKind.HoistedConst
473 + : binding.kind === 'let'
474 + ? InstructionKind.HoistedLet
475 + : assertExhaustive(binding.kind, 'Unexpected binding kind');
476 lowerValueToTemporary(builder, {
477 kind: 'DeclareContext',
478 lvalue: {
469 - kind: InstructionKind.HoistedConst,
479 + kind,
480 place,
481 },
482 loc: id.node.loc ?? GeneratedSource,
compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
+7 -1
@@ -741,6 +741,9 @@ export enum InstructionKind {
741
742 // hoisted const declarations
743 HoistedConst = 'HoistedConst',
744 +
745 + // hoisted const declarations
746 + HoistedLet = 'HoistedLet',
747 }
748
749 function _staticInvariantInstructionValueHasLocation(
@@ -858,7 +861,10 @@ export type InstructionValue =
861 | {
862 kind: 'DeclareContext';
863 lvalue: {
861 - kind: InstructionKind.Let | InstructionKind.HoistedConst;
864 + kind:
865 + | InstructionKind.Let
866 + | InstructionKind.HoistedConst
867 + | InstructionKind.HoistedLet;
868 place: Place;
869 };
870 loc: SourceLocation;
compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts
+3
@@ -760,6 +760,9 @@ export function printLValue(lval: LValue): string {
760 case InstructionKind.HoistedConst: {
761 return `HoistedConst ${lvalue}$`;
762 }
763 + case InstructionKind.HoistedLet: {
764 + return `HoistedLet ${lvalue}$`;
765 + }
766 default: {
767 assertExhaustive(lval.kind, `Unexpected lvalue kind \`${lval.kind}\``);
768 }
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts
+23
@@ -994,6 +994,13 @@ function codegenTerminal(
994 loc: iterableItem.loc,
995 suggestions: null,
996 });
997 + case InstructionKind.HoistedLet:
998 + CompilerError.invariant(false, {
999 + reason: 'Unexpected HoistedLet variable in for..in collection',
1000 + description: null,
1001 + loc: iterableItem.loc,
1002 + suggestions: null,
1003 + });
1004 default:
1005 assertExhaustive(
1006 iterableItem.value.lvalue.kind,
@@ -1089,6 +1096,13 @@ function codegenTerminal(
1096 loc: iterableItem.loc,
1097 suggestions: null,
1098 });
1099 + case InstructionKind.HoistedLet:
1100 + CompilerError.invariant(false, {
1101 + reason: 'Unexpected HoistedLet variable in for..of collection',
1102 + description: null,
1103 + loc: iterableItem.loc,
1104 + suggestions: null,
1105 + });
1106 default:
1107 assertExhaustive(
1108 iterableItem.value.lvalue.kind,
@@ -1289,6 +1303,15 @@ function codegenInstructionNullable(
1303 case InstructionKind.Catch: {
1304 return t.emptyStatement();
1305 }
1306 + case InstructionKind.HoistedLet: {
1307 + CompilerError.invariant(false, {
1308 + reason:
1309 + 'Expected HoistedLet to have been pruned in PruneHoistedContexts',
1310 + description: null,
1311 + loc: instr.loc,
1312 + suggestions: null,
1313 + });
1314 + }
1315 case InstructionKind.HoistedConst: {
1316 CompilerError.invariant(false, {
1317 reason:
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts
+21 -4
@@ -23,11 +23,11 @@ import {
23 * original instruction kind.
24 */
25 export function pruneHoistedContexts(fn: ReactiveFunction): void {
26 - const hoistedIdentifiers: HoistedIdentifiers = new Set();
26 + const hoistedIdentifiers: HoistedIdentifiers = new Map();
27 visitReactiveFunction(fn, new Visitor(), hoistedIdentifiers);
28 }
29
30 -type HoistedIdentifiers = Set<DeclarationId>;
30 +type HoistedIdentifiers = Map<DeclarationId, InstructionKind>;
31
32 class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
33 override transformInstruction(
@@ -39,7 +39,21 @@ class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
39 instruction.value.kind === 'DeclareContext' &&
40 instruction.value.lvalue.kind === 'HoistedConst'
41 ) {
42 - state.add(instruction.value.lvalue.place.identifier.declarationId);
42 + state.set(
43 + instruction.value.lvalue.place.identifier.declarationId,
44 + InstructionKind.Const,
45 + );
46 + return {kind: 'remove'};
47 + }
48 +
49 + if (
50 + instruction.value.kind === 'DeclareContext' &&
51 + instruction.value.lvalue.kind === 'HoistedLet'
52 + ) {
53 + state.set(
54 + instruction.value.lvalue.place.identifier.declarationId,
55 + InstructionKind.Let,
56 + );
57 return {kind: 'remove'};
58 }
59
@@ -47,6 +61,9 @@ class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
61 instruction.value.kind === 'StoreContext' &&
62 state.has(instruction.value.lvalue.place.identifier.declarationId)
63 ) {
64 + const kind = state.get(
65 + instruction.value.lvalue.place.identifier.declarationId,
66 + )!;
67 return {
68 kind: 'replace',
69 value: {
@@ -57,7 +74,7 @@ class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
74 ...instruction.value,
75 lvalue: {
76 ...instruction.value.lvalue,
60 - kind: InstructionKind.Const,
77 + kind,
78 },
79 type: null,
80 kind: 'StoreLocal',
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts
+10
@@ -130,6 +130,16 @@ function getContextReassignment(
130 */
131 contextVariables.add(value.lvalue.place.identifier.id);
132 }
133 + const reassignment = reassigningFunctions.get(
134 + value.value.identifier.id,
135 + );
136 + if (reassignment !== undefined) {
137 + reassigningFunctions.set(
138 + value.lvalue.place.identifier.id,
139 + reassignment,
140 + );
141 + reassigningFunctions.set(lvalue.identifier.id, reassignment);
142 + }
143 break;
144 }
145 default: {
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.function-expression-references-variable-its-assigned-to.expect.md renamed
+1 -1
@@ -18,7 +18,7 @@ function Component() {
18 1 | function Component() {
19 2 | let callback = () => {
20 > 3 | callback = null;
21 - | ^^^^^^^^^^^^^^^ Todo: Handle non-const declarations for hoisting. variable "callback" declared with let (3:3)
21 + | ^^^^^^^^ InvalidReact: Reassigning a variable after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `callback` cannot be reassigned after render (3:3)
22 4 | };
23 5 | return <div onClick={callback} />;
24 6 | }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.function-expression-references-variable-its-assigned-to.js renamed
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.mutate-captured-arg-separately.expect.md deleted
-31
@@ -1,31 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -// Let's not support identifiers defined after use for now.
6 -function component(a) {
7 - let y = function () {
8 - m(x);
9 - };
10 -
11 - let x = {a};
12 - m(x);
13 - return y;
14 -}
15 -
16 -```
17 -
18 -
19 -## Error
20 -
21 -```
22 - 2 | function component(a) {
23 - 3 | let y = function () {
24 -> 4 | m(x);
25 - | ^^^^ Todo: Handle non-const declarations for hoisting. variable "x" declared with let (4:4)
26 - 5 | };
27 - 6 |
28 - 7 | let x = {a};
29 -```
30 -
31 -
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.mutate-captured-arg-separately.js deleted
-10
@@ -1,10 +0,0 @@
1 -// Let's not support identifiers defined after use for now.
2 -function component(a) {
3 - let y = function () {
4 - m(x);
5 - };
6 -
7 - let x = {a};
8 - m(x);
9 - return y;
10 -}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-function-expression-references-later-variable-declaration.expect.md
+1 -1
@@ -20,7 +20,7 @@ function Component() {
20 1 | function Component() {
21 2 | let callback = () => {
22 > 3 | onClick = () => {};
23 - | ^^^^^^^^^^^^^^^^^^ Todo: Handle non-const declarations for hoisting. variable "onClick" declared with let (3:3)
23 + | ^^^^^^^ InvalidReact: Reassigning a variable after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `onClick` cannot be reassigned after render (3:3)
24 4 | };
25 5 | let onClick;
26 6 |
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-nested-let-declaration-2.expect.md new
+62
@@ -0,0 +1,62 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function hoisting(cond) {
6 + let items = [];
7 + if (cond) {
8 + let foo = () => {
9 + items.push(bar());
10 + };
11 + let bar = () => true;
12 + foo();
13 + }
14 + return items;
15 +}
16 +
17 +export const FIXTURE_ENTRYPOINT = {
18 + fn: hoisting,
19 + params: [true],
20 + isComponent: false,
21 +};
22 +
23 +```
24 +
25 +## Code
26 +
27 +```javascript
28 +import { c as _c } from "react/compiler-runtime";
29 +function hoisting(cond) {
30 + const $ = _c(2);
31 + let items;
32 + if ($[0] !== cond) {
33 + items = [];
34 + if (cond) {
35 + const foo = () => {
36 + items.push(bar());
37 + };
38 +
39 + let bar = _temp;
40 + foo();
41 + }
42 + $[0] = cond;
43 + $[1] = items;
44 + } else {
45 + items = $[1];
46 + }
47 + return items;
48 +}
49 +function _temp() {
50 + return true;
51 +}
52 +
53 +export const FIXTURE_ENTRYPOINT = {
54 + fn: hoisting,
55 + params: [true],
56 + isComponent: false,
57 +};
58 +
59 +```
60 +
61 +### Eval output
62 +(kind: ok) [true]
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-nested-let-declaration-2.js new
+17
@@ -0,0 +1,17 @@
1 +function hoisting(cond) {
2 + let items = [];
3 + if (cond) {
4 + let foo = () => {
5 + items.push(bar());
6 + };
7 + let bar = () => true;
8 + foo();
9 + }
10 + return items;
11 +}
12 +
13 +export const FIXTURE_ENTRYPOINT = {
14 + fn: hoisting,
15 + params: [true],
16 + isComponent: false,
17 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-nested-let-declaration.expect.md new
+65
@@ -0,0 +1,65 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function hoisting() {
6 + let qux = () => {
7 + let result;
8 + {
9 + result = foo();
10 + }
11 + return result;
12 + };
13 + let foo = () => {
14 + return bar + baz;
15 + };
16 + let bar = 3;
17 + const baz = 2;
18 + return qux(); // OK: called outside of TDZ
19 +}
20 +
21 +export const FIXTURE_ENTRYPOINT = {
22 + fn: hoisting,
23 + params: [],
24 + isComponent: false,
25 +};
26 +
27 +```
28 +
29 +## Code
30 +
31 +```javascript
32 +import { c as _c } from "react/compiler-runtime";
33 +function hoisting() {
34 + const $ = _c(1);
35 + let t0;
36 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
37 + const qux = () => {
38 + let result;
39 +
40 + result = foo();
41 + return result;
42 + };
43 +
44 + let foo = () => bar + baz;
45 +
46 + let bar = 3;
47 + const baz = 2;
48 + t0 = qux();
49 + $[0] = t0;
50 + } else {
51 + t0 = $[0];
52 + }
53 + return t0;
54 +}
55 +
56 +export const FIXTURE_ENTRYPOINT = {
57 + fn: hoisting,
58 + params: [],
59 + isComponent: false,
60 +};
61 +
62 +```
63 +
64 +### Eval output
65 +(kind: ok) 5
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-nested-let-declaration.js new
+21
@@ -0,0 +1,21 @@
1 +function hoisting() {
2 + let qux = () => {
3 + let result;
4 + {
5 + result = foo();
6 + }
7 + return result;
8 + };
9 + let foo = () => {
10 + return bar + baz;
11 + };
12 + let bar = 3;
13 + const baz = 2;
14 + return qux(); // OK: called outside of TDZ
15 +}
16 +
17 +export const FIXTURE_ENTRYPOINT = {
18 + fn: hoisting,
19 + params: [],
20 + isComponent: false,
21 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-simple-let-declaration.expect.md new
+51
@@ -0,0 +1,51 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function hoisting() {
6 + let foo = () => {
7 + return bar + baz;
8 + };
9 + let bar = 3;
10 + let baz = 2;
11 + return foo(); // OK: called outside of TDZ for bar/baz
12 +}
13 +
14 +export const FIXTURE_ENTRYPOINT = {
15 + fn: hoisting,
16 + params: [],
17 + isComponent: false,
18 +};
19 +
20 +```
21 +
22 +## Code
23 +
24 +```javascript
25 +import { c as _c } from "react/compiler-runtime";
26 +function hoisting() {
27 + const $ = _c(1);
28 + let t0;
29 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
30 + const foo = () => bar + baz;
31 +
32 + let bar = 3;
33 + let baz = 2;
34 + t0 = foo();
35 + $[0] = t0;
36 + } else {
37 + t0 = $[0];
38 + }
39 + return t0;
40 +}
41 +
42 +export const FIXTURE_ENTRYPOINT = {
43 + fn: hoisting,
44 + params: [],
45 + isComponent: false,
46 +};
47 +
48 +```
49 +
50 +### Eval output
51 +(kind: ok) 5
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hoisting-simple-let-declaration.js new
+14
@@ -0,0 +1,14 @@
1 +function hoisting() {
2 + let foo = () => {
3 + return bar + baz;
4 + };
5 + let bar = 3;
6 + let baz = 2;
7 + return foo(); // OK: called outside of TDZ for bar/baz
8 +}
9 +
10 +export const FIXTURE_ENTRYPOINT = {
11 + fn: hoisting,
12 + params: [],
13 + isComponent: false,
14 +};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mutate-captured-arg-separately.expect.md new
+56
@@ -0,0 +1,56 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function component(a) {
6 + let y = function () {
7 + m(x);
8 + };
9 +
10 + let x = {a};
11 + m(x);
12 + return y;
13 +}
14 +
15 +function m(x) {}
16 +
17 +export const FIXTURE_ENTRYPOINT = {
18 + fn: component,
19 + params: [{name: 'Jason'}],
20 +};
21 +
22 +```
23 +
24 +## Code
25 +
26 +```javascript
27 +import { c as _c } from "react/compiler-runtime";
28 +function component(a) {
29 + const $ = _c(2);
30 + let y;
31 + if ($[0] !== a) {
32 + y = function () {
33 + m(x);
34 + };
35 +
36 + let x = { a };
37 + m(x);
38 + $[0] = a;
39 + $[1] = y;
40 + } else {
41 + y = $[1];
42 + }
43 + return y;
44 +}
45 +
46 +function m(x) {}
47 +
48 +export const FIXTURE_ENTRYPOINT = {
49 + fn: component,
50 + params: [{ name: "Jason" }],
51 +};
52 +
53 +```
54 +
55 +### Eval output
56 +(kind: ok) "[[ function params=0 ]]"
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mutate-captured-arg-separately.js new
+16
@@ -0,0 +1,16 @@
1 +function component(a) {
2 + let y = function () {
3 + m(x);
4 + };
5 +
6 + let x = {a};
7 + m(x);
8 + return y;
9 +}
10 +
11 +function m(x) {}
12 +
13 +export const FIXTURE_ENTRYPOINT = {
14 + fn: component,
15 + params: [{name: 'Jason'}],
16 +};