Fix destructuring with mixed local/scope declarations
Fixes T176436488. The logic for rewriting Destructure instructions was correct, but the visitor implementation was accidentally dropping subsequent Destructure instructions within a block after encountering one that needed a rewrite. Switching to use the transform infra (added after this pass was written) fixes it.
Joe Savona committed
Feb 5, 2024 at 22:29 UTC
af6e837b60fcdf1ae77e119f63afc4a6cff41459
5 files changed
+352
-34
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts
+26
-34
@@ -11,13 +11,17 @@ import {
11
IdentifierId,
12
InstructionKind,
13
Place,
14
- ReactiveBlock,
14
ReactiveFunction,
15
ReactiveInstruction,
16
ReactiveScopeBlock,
17
+ ReactiveStatement,
18
} from "../HIR";
19
import { eachPatternOperand, mapPatternOperands } from "../HIR/visitors";
20
-import { ReactiveFunctionTransform, visitReactiveFunction } from "./visitors";
20
+import {
21
+ ReactiveFunctionTransform,
22
+ Transformed,
23
+ visitReactiveFunction,
24
+} from "./visitors";
25
26
/*
27
* Destructuring statements may sometimes define some variables which are declared by the scope,
@@ -92,41 +96,29 @@ class Visitor extends ReactiveFunctionTransform<State> {
96
this.traverseScope(scope, state);
97
}
98
95
- override visitBlock(block: ReactiveBlock, state: State): void {
96
- // Traverse first to transform inner items
97
- this.traverseBlock(block, state);
99
+ override transformInstruction(
100
+ instruction: ReactiveInstruction,
101
+ state: State
102
+ ): Transformed<ReactiveStatement> {
103
+ this.visitInstruction(instruction, state);
104
99
- // Then transform any mixed destructuring instructions
100
- let nextBlock: ReactiveBlock | null = null;
101
- for (let i = 0; i < block.length; i++) {
102
- const instr = block[i];
103
- if (
104
- instr.kind === "instruction" &&
105
- instr.instruction.value.kind === "Destructure"
106
- ) {
107
- const transformed = transformDestructuring(
108
- state,
109
- instr.instruction,
110
- instr.instruction.value
111
- );
112
- if (transformed) {
113
- nextBlock ??= block.slice(0, i);
114
- transformed.forEach((instruction) => {
115
- nextBlock?.push({
116
- kind: "instruction",
117
- instruction,
118
- });
119
- });
120
- continue;
121
- }
122
- } else if (nextBlock !== null) {
123
- nextBlock.push(instr);
105
+ if (instruction.value.kind === "Destructure") {
106
+ const transformed = transformDestructuring(
107
+ state,
108
+ instruction,
109
+ instruction.value
110
+ );
111
+ if (transformed) {
112
+ return {
113
+ kind: "replace-many",
114
+ value: transformed.map((instruction) => ({
115
+ kind: "instruction",
116
+ instruction,
117
+ })),
118
+ };
119
}
120
}
126
- if (nextBlock !== null) {
127
- block.length = 0;
128
- block.push(...nextBlock);
129
- }
121
+ return { kind: "keep" };
122
}
123
}
124
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/sequential-destructuring-assignment-to-scope-declarations.expect.md
new
+129
@@ -0,0 +1,129 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { identity } from "shared-runtime";
6
+
7
+function Component(statusName) {
8
+ const { status, text } = foo(statusName);
9
+ const { bg, color } = getStyles(status);
10
+ return (
11
+ <div className={identity(bg)}>
12
+ <span className={identity(color)}>{[text]}</span>
13
+ </div>
14
+ );
15
+}
16
+
17
+function foo(name) {
18
+ return {
19
+ status: `<status>`,
20
+ text: `${name}!`,
21
+ };
22
+}
23
+
24
+function getStyles(status) {
25
+ return {
26
+ bg: "#eee8d5",
27
+ color: "#657b83",
28
+ };
29
+}
30
+
31
+export const FIXTURE_ENTRYPOINT = {
32
+ fn: Component,
33
+ params: ["Mofei"],
34
+};
35
+
36
+```
37
+
38
+## Code
39
+
40
+```javascript
41
+import { unstable_useMemoCache as useMemoCache } from "react";
42
+import { identity } from "shared-runtime";
43
+
44
+function Component(statusName) {
45
+ const $ = useMemoCache(12);
46
+ let text;
47
+ let t0;
48
+ let t1;
49
+ if ($[0] !== statusName) {
50
+ const { status, text: t47 } = foo(statusName);
51
+ text = t47;
52
+ const { bg, color } = getStyles(status);
53
+
54
+ t1 = identity(bg);
55
+ t0 = identity(color);
56
+ $[0] = statusName;
57
+ $[1] = text;
58
+ $[2] = t0;
59
+ $[3] = t1;
60
+ } else {
61
+ text = $[1];
62
+ t0 = $[2];
63
+ t1 = $[3];
64
+ }
65
+ let t2;
66
+ if ($[4] !== text) {
67
+ t2 = [text];
68
+ $[4] = text;
69
+ $[5] = t2;
70
+ } else {
71
+ t2 = $[5];
72
+ }
73
+ let t3;
74
+ if ($[6] !== t0 || $[7] !== t2) {
75
+ t3 = <span className={t0}>{t2}</span>;
76
+ $[6] = t0;
77
+ $[7] = t2;
78
+ $[8] = t3;
79
+ } else {
80
+ t3 = $[8];
81
+ }
82
+ let t4;
83
+ if ($[9] !== t1 || $[10] !== t3) {
84
+ t4 = <div className={t1}>{t3}</div>;
85
+ $[9] = t1;
86
+ $[10] = t3;
87
+ $[11] = t4;
88
+ } else {
89
+ t4 = $[11];
90
+ }
91
+ return t4;
92
+}
93
+
94
+function foo(name) {
95
+ const $ = useMemoCache(2);
96
+
97
+ const t0 = `${name}!`;
98
+ let t1;
99
+ if ($[0] !== t0) {
100
+ t1 = { status: `<status>`, text: t0 };
101
+ $[0] = t0;
102
+ $[1] = t1;
103
+ } else {
104
+ t1 = $[1];
105
+ }
106
+ return t1;
107
+}
108
+
109
+function getStyles(status) {
110
+ const $ = useMemoCache(1);
111
+ let t0;
112
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
113
+ t0 = { bg: "#eee8d5", color: "#657b83" };
114
+ $[0] = t0;
115
+ } else {
116
+ t0 = $[0];
117
+ }
118
+ return t0;
119
+}
120
+
121
+export const FIXTURE_ENTRYPOINT = {
122
+ fn: Component,
123
+ params: ["Mofei"],
124
+};
125
+
126
+```
127
+
128
+### Eval output
129
+(kind: ok) <div class="#eee8d5"><span class="#657b83">Mofei!</span></div>
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/sequential-destructuring-assignment-to-scope-declarations.js
new
+30
@@ -0,0 +1,30 @@
1
+import { identity } from "shared-runtime";
2
+
3
+function Component(statusName) {
4
+ const { status, text } = foo(statusName);
5
+ const { bg, color } = getStyles(status);
6
+ return (
7
+ <div className={identity(bg)}>
8
+ <span className={identity(color)}>{[text]}</span>
9
+ </div>
10
+ );
11
+}
12
+
13
+function foo(name) {
14
+ return {
15
+ status: `<status>`,
16
+ text: `${name}!`,
17
+ };
18
+}
19
+
20
+function getStyles(status) {
21
+ return {
22
+ bg: "#eee8d5",
23
+ color: "#657b83",
24
+ };
25
+}
26
+
27
+export const FIXTURE_ENTRYPOINT = {
28
+ fn: Component,
29
+ params: ["Mofei"],
30
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/sequential-destructuring-both-mixed-local-and-scope-declaration.expect.md
new
+134
@@ -0,0 +1,134 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { identity } from "shared-runtime";
6
+
7
+function Component(statusName) {
8
+ // status is local, text is a scope declaration
9
+ const { status, text } = foo(statusName);
10
+ // color is local, font is a scope declaration
11
+ const { color, font } = getStyles(status);
12
+ // bg is a declaration
13
+ const bg = identity(color);
14
+ return (
15
+ <div className={bg}>
16
+ <span className={font}>{[text]}</span>
17
+ </div>
18
+ );
19
+}
20
+function foo(name) {
21
+ return {
22
+ status: `<status>`,
23
+ text: `${name}!`,
24
+ };
25
+}
26
+
27
+function getStyles(status) {
28
+ return {
29
+ font: "comic-sans",
30
+ color: "#657b83",
31
+ };
32
+}
33
+
34
+export const FIXTURE_ENTRYPOINT = {
35
+ fn: Component,
36
+ params: ["Sathya"],
37
+};
38
+
39
+```
40
+
41
+## Code
42
+
43
+```javascript
44
+import { unstable_useMemoCache as useMemoCache } from "react";
45
+import { identity } from "shared-runtime";
46
+
47
+function Component(statusName) {
48
+ const $ = useMemoCache(12);
49
+ let t0;
50
+ let text;
51
+ let font;
52
+ if ($[0] !== statusName) {
53
+ const { status, text: t49 } = foo(statusName);
54
+ text = t49;
55
+
56
+ const { color, font: t50 } = getStyles(status);
57
+ font = t50;
58
+
59
+ t0 = identity(color);
60
+ $[0] = statusName;
61
+ $[1] = t0;
62
+ $[2] = text;
63
+ $[3] = font;
64
+ } else {
65
+ t0 = $[1];
66
+ text = $[2];
67
+ font = $[3];
68
+ }
69
+ const bg = t0;
70
+ let t1;
71
+ if ($[4] !== text) {
72
+ t1 = [text];
73
+ $[4] = text;
74
+ $[5] = t1;
75
+ } else {
76
+ t1 = $[5];
77
+ }
78
+ let t2;
79
+ if ($[6] !== font || $[7] !== t1) {
80
+ t2 = <span className={font}>{t1}</span>;
81
+ $[6] = font;
82
+ $[7] = t1;
83
+ $[8] = t2;
84
+ } else {
85
+ t2 = $[8];
86
+ }
87
+ let t3;
88
+ if ($[9] !== bg || $[10] !== t2) {
89
+ t3 = <div className={bg}>{t2}</div>;
90
+ $[9] = bg;
91
+ $[10] = t2;
92
+ $[11] = t3;
93
+ } else {
94
+ t3 = $[11];
95
+ }
96
+ return t3;
97
+}
98
+
99
+function foo(name) {
100
+ const $ = useMemoCache(2);
101
+
102
+ const t0 = `${name}!`;
103
+ let t1;
104
+ if ($[0] !== t0) {
105
+ t1 = { status: `<status>`, text: t0 };
106
+ $[0] = t0;
107
+ $[1] = t1;
108
+ } else {
109
+ t1 = $[1];
110
+ }
111
+ return t1;
112
+}
113
+
114
+function getStyles(status) {
115
+ const $ = useMemoCache(1);
116
+ let t0;
117
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
118
+ t0 = { font: "comic-sans", color: "#657b83" };
119
+ $[0] = t0;
120
+ } else {
121
+ t0 = $[0];
122
+ }
123
+ return t0;
124
+}
125
+
126
+export const FIXTURE_ENTRYPOINT = {
127
+ fn: Component,
128
+ params: ["Sathya"],
129
+};
130
+
131
+```
132
+
133
+### Eval output
134
+(kind: ok) <div class="#657b83"><span class="comic-sans">Sathya!</span></div>
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/sequential-destructuring-both-mixed-local-and-scope-declaration.js
new
+33
@@ -0,0 +1,33 @@
1
+import { identity } from "shared-runtime";
2
+
3
+function Component(statusName) {
4
+ // status is local, text is a scope declaration
5
+ const { status, text } = foo(statusName);
6
+ // color is local, font is a scope declaration
7
+ const { color, font } = getStyles(status);
8
+ // bg is a declaration
9
+ const bg = identity(color);
10
+ return (
11
+ <div className={bg}>
12
+ <span className={font}>{[text]}</span>
13
+ </div>
14
+ );
15
+}
16
+function foo(name) {
17
+ return {
18
+ status: `<status>`,
19
+ text: `${name}!`,
20
+ };
21
+}
22
+
23
+function getStyles(status) {
24
+ return {
25
+ font: "comic-sans",
26
+ color: "#657b83",
27
+ };
28
+}
29
+
30
+export const FIXTURE_ENTRYPOINT = {
31
+ fn: Component,
32
+ params: ["Sathya"],
33
+};