Fix variable-resolution hoisting issues
Fixes one category of bugs with const hoisting. The algorithm finds all consts that need to be hoisted, then looks through the statements of a block to find the first statement which references that const, delaying the emission of the HoistedConst instruction until its actually used. To determine if a statement references a const we find every identifier in the statement and check if its binding is one of the hoisted bindings. There's a very small bug here: when we resolve the binding of each identifier, we need to resolve it in its own scope. We're currently resolving these identifiers agains the outer block statement's scope, which can cause us to misattribute identifiers when there is shadowing: ``` const items = props.items.map(x => x); // we scan this statement, resolve 'x' in the block statement scope, and mis-attribute it to the outer x. const x = 42; // (1) this x is a candidate for hoisting, so the binding is in the set of hoisted consts ```
Joe Savona committed
Nov 15, 2023 at 16:55 UTC
df42058237fdc0420116529a257f9550bce3312f
10 files changed
+159
-56
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+1
-1
@@ -369,7 +369,7 @@ function lowerStatement(
369
*/
370
s.traverse({
371
Identifier(id: NodePath<t.Identifier>) {
372
- const binding = stmt.scope.getBinding(id.node.name);
372
+ const binding = id.scope.getBinding(id.node.name);
373
if (binding != null && hoistableBindings.has(binding)) {
374
if (
375
id.parentPath.isVariableDeclarator() ||
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-hoisting.expect.md
deleted
-26
@@ -1,26 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-function Component(props) {
6
- const wat = () => {
7
- const pathname = "wat";
8
- pathname;
9
- };
10
-
11
- const pathname = props.wat;
12
- const deeplinkItemId = pathname ? itemID : null;
13
-
14
- return <button onClick={() => wat()}>{deeplinkItemId}</button>;
15
-}
16
-
17
-```
18
-
19
-
20
-## Error
21
-
22
-```
23
-[ReactForget] Invariant: [hoisting] Expected value kind to be initialized. read pathname_0$12 (8:8)
24
-```
25
-
26
-
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.repro-hoisting-variable-collision.expect.md
deleted
-20
@@ -1,20 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-function Component(props) {
6
- const items = props.items.map((x) => x);
7
- const x = 42;
8
- return [x, items];
9
-}
10
-
11
-```
12
-
13
-
14
-## Error
15
-
16
-```
17
-[ReactForget] Invariant: [hoisting] Expected value kind to be initialized. read x_0$13 (4:4)
18
-```
19
-
20
-
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.repro-hoisting-variable-collision.js
deleted
-5
@@ -1,5 +0,0 @@
1
-function Component(props) {
2
- const items = props.items.map((x) => x);
3
- const x = 42;
4
- return [x, items];
5
-}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hoisting-nested-block-statements.expect.md
renamed
+15
-4
@@ -20,11 +20,22 @@ export const FIXTURE_ENTRYPOINT = {
20
21
```
22
23
+## Code
24
24
-## Error
25
+```javascript
26
+function hoisting(cond) {
27
+ if (cond) {
28
+ foo(1);
29
+ }
30
+
31
+ foo(2);
32
+}
33
+
34
+export const FIXTURE_ENTRYPOINT = {
35
+ fn: hoisting,
36
+ params: [false],
37
+ isComponent: false,
38
+};
39
40
```
27
-[ReactForget] Invariant: [hoisting] Expected value kind to be initialized. read x_0$10 (8:8)
28
-```
29
-
41
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hoisting-nested-block-statements.js
renamed
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-hoisting-variable-collision.expect.md
new
+59
@@ -0,0 +1,59 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const items = props.items.map((x) => x);
7
+ const x = 42;
8
+ return [x, items];
9
+}
10
+
11
+export const FIXTURE_ENTRYPOINT = {
12
+ fn: Component,
13
+ params: [{ items: [0, 42, null, undefined, { object: true }] }],
14
+ isComponent: "Component",
15
+};
16
+
17
+```
18
+
19
+## Code
20
+
21
+```javascript
22
+import { unstable_useMemoCache as useMemoCache } from "react";
23
+function Component(props) {
24
+ const $ = useMemoCache(5);
25
+ let t1;
26
+ if ($[0] !== props.items) {
27
+ let t0;
28
+ if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
29
+ t0 = (x) => x;
30
+ $[2] = t0;
31
+ } else {
32
+ t0 = $[2];
33
+ }
34
+ t1 = props.items.map(t0);
35
+ $[0] = props.items;
36
+ $[1] = t1;
37
+ } else {
38
+ t1 = $[1];
39
+ }
40
+ const items = t1;
41
+ let t2;
42
+ if ($[3] !== items) {
43
+ t2 = [42, items];
44
+ $[3] = items;
45
+ $[4] = t2;
46
+ } else {
47
+ t2 = $[4];
48
+ }
49
+ return t2;
50
+}
51
+
52
+export const FIXTURE_ENTRYPOINT = {
53
+ fn: Component,
54
+ params: [{ items: [0, 42, null, undefined, { object: true }] }],
55
+ isComponent: "Component",
56
+};
57
+
58
+```
59
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-hoisting-variable-collision.js
new
+11
@@ -0,0 +1,11 @@
1
+function Component(props) {
2
+ const items = props.items.map((x) => x);
3
+ const x = 42;
4
+ return [x, items];
5
+}
6
+
7
+export const FIXTURE_ENTRYPOINT = {
8
+ fn: Component,
9
+ params: [{ items: [0, 42, null, undefined, { object: true }] }],
10
+ isComponent: "Component",
11
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-hoisting.expect.md
new
+67
@@ -0,0 +1,67 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const wat = () => {
7
+ const pathname = "wat";
8
+ pathname;
9
+ };
10
+
11
+ const pathname = props.wat;
12
+ const deeplinkItemId = pathname ? itemID : null;
13
+
14
+ return <button onClick={() => wat()}>{deeplinkItemId}</button>;
15
+}
16
+
17
+export const FIXTURE_ENTRYPOINT = {
18
+ fn: Component,
19
+ params: [{ wat: "/dev/null", itemID: 42 }],
20
+ isComponent: "Component",
21
+};
22
+
23
+```
24
+
25
+## Code
26
+
27
+```javascript
28
+import { unstable_useMemoCache as useMemoCache } from "react";
29
+function Component(props) {
30
+ const $ = useMemoCache(4);
31
+ let t0;
32
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
33
+ t0 = () => {};
34
+ $[0] = t0;
35
+ } else {
36
+ t0 = $[0];
37
+ }
38
+ const wat = t0;
39
+
40
+ const pathname_0 = props.wat;
41
+ const deeplinkItemId = pathname_0 ? itemID : null;
42
+ let t1;
43
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
44
+ t1 = () => wat();
45
+ $[1] = t1;
46
+ } else {
47
+ t1 = $[1];
48
+ }
49
+ let t2;
50
+ if ($[2] !== deeplinkItemId) {
51
+ t2 = <button onClick={t1}>{deeplinkItemId}</button>;
52
+ $[2] = deeplinkItemId;
53
+ $[3] = t2;
54
+ } else {
55
+ t2 = $[3];
56
+ }
57
+ return t2;
58
+}
59
+
60
+export const FIXTURE_ENTRYPOINT = {
61
+ fn: Component,
62
+ params: [{ wat: "/dev/null", itemID: 42 }],
63
+ isComponent: "Component",
64
+};
65
+
66
+```
67
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-hoisting.js
renamed
+6
@@ -9,3 +9,9 @@ function Component(props) {
9
10
return <button onClick={() => wat()}>{deeplinkItemId}</button>;
11
}
12
+
13
+export const FIXTURE_ENTRYPOINT = {
14
+ fn: Component,
15
+ params: [{ wat: "/dev/null", itemID: 42 }],
16
+ isComponent: "Component",
17
+};