[fix] JSXElement identifiers now included in lambda capture deps
--- `gatherCapturedDeps` previously did not visit JSXElements, so Forget did not read any local JSX identifiers as dependencies (in lambdas)
Mofei Zhang committed
Oct 4, 2023 at 14:01 UTC
1ec1a0ceb84de690e50dfd45fe476302ea954c59
6 files changed
+251
-83
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+118
-55
@@ -3457,78 +3457,141 @@ function gatherCapturedDeps(
3457
to: componentScope,
3458
});
3459
3460
- function visit(path: NodePath<Expression>): void {
3461
- // Babel has a bug where it doesn't visit the LHS of an
3462
- // AssignmentExpression if it's an Identifier. Work around it by explicitly
3463
- // visiting it.
3464
- if (path.isAssignmentExpression()) {
3465
- const left = path.get("left");
3466
- if (left.isIdentifier()) {
3467
- visit(left);
3468
- }
3469
- return;
3460
+ function addCapturedId(bindingIdentifier: t.Identifier): number {
3461
+ if (!capturedIds.has(bindingIdentifier)) {
3462
+ const index = capturedIds.size;
3463
+ capturedIds.set(bindingIdentifier, index);
3464
+ return index;
3465
+ } else {
3466
+ return capturedIds.get(bindingIdentifier)!;
3467
}
3468
+ }
3469
3472
- let obj = path;
3473
- while (obj.isMemberExpression()) {
3474
- obj = obj.get("object");
3475
- }
3470
+ function handleMaybeDependency(
3471
+ path:
3472
+ | NodePath<t.MemberExpression>
3473
+ | NodePath<t.Identifier>
3474
+ | NodePath<t.JSXOpeningElement>
3475
+ ): void {
3476
+ // Base context variable to depend on
3477
+ let baseIdentifier: NodePath<t.Identifier | t.JSXIdentifier>;
3478
+ // Base expression to depend on, which (for now) may contain non side-effectful
3479
+ // member expressions
3480
+ let dependency:
3481
+ | NodePath<t.MemberExpression>
3482
+ | NodePath<t.Identifier>
3483
+ | NodePath<t.JSXIdentifier>;
3484
+ if (path.isJSXOpeningElement()) {
3485
+ const name = path.get("name");
3486
+ if (!(name.isJSXMemberExpression() || name.isJSXIdentifier())) {
3487
+ // TODO: should JSX namespaced names be handled here as well?
3488
+ return;
3489
+ }
3490
+ let current: NodePath<t.JSXMemberExpression | t.JSXIdentifier> = name;
3491
+ while (current.isJSXMemberExpression()) {
3492
+ current = current.get("object");
3493
+ }
3494
+ invariant(
3495
+ current.isJSXIdentifier(),
3496
+ "Invalid logic in gatherCapturedDeps"
3497
+ );
3498
+ baseIdentifier = current;
3499
+ dependency = current;
3500
+ } else if (path.isMemberExpression()) {
3501
+ // Calculate baseIdentifier
3502
+ let current: NodePath<Expression> = path;
3503
+ while (current.isMemberExpression()) {
3504
+ current = current.get("object");
3505
+ }
3506
+ if (!current.isIdentifier()) {
3507
+ return;
3508
+ }
3509
+ baseIdentifier = current;
3510
3477
- if (!obj.isIdentifier()) {
3478
- return;
3511
+ // Get the expression to depend on, which may involve PropertyLoads
3512
+ // for member expressions
3513
+ current =
3514
+ path.parent.type === "CallExpression" &&
3515
+ path.parent.callee === path.node
3516
+ ? path.get("object")
3517
+ : path;
3518
+ while (current.isMemberExpression() && current.node.computed) {
3519
+ // computed nodes may contain side-effectful subexpressions
3520
+ current = current.get("object");
3521
+ }
3522
+ invariant(
3523
+ current.isMemberExpression() || current.isIdentifier(),
3524
+ "Internal invariant broken in BuildHIR, unexpected type for capturedDep"
3525
+ );
3526
+ dependency = current;
3527
+ } else {
3528
+ baseIdentifier = path;
3529
+ dependency = path;
3530
}
3531
3481
- const binding = obj.scope.getBinding(obj.node.name);
3532
+ /**
3533
+ * Skip dependency path, as we already tried to recursively add it (+ all subexpressions)
3534
+ * as a dependency.
3535
+ */
3536
+ dependency.skip();
3537
+
3538
+ /**
3539
+ * Add the base identifier binding as a dependency.
3540
+ */
3541
+ const binding = baseIdentifier.scope.getBinding(baseIdentifier.node.name);
3542
if (binding === undefined || !pureScopes.has(binding.scope)) {
3543
return;
3544
}
3485
-
3486
- if (path.isMemberExpression()) {
3487
- // For CallExpression, we need to depend on the receiver, not the
3488
- // function itself.
3489
- if (
3490
- path.parent.type === "CallExpression" &&
3491
- path.parent.callee === path.node
3492
- ) {
3493
- path = path.get("object");
3494
- }
3495
-
3496
- // Skip the computed part of the member expression.
3497
- while (path.isMemberExpression() && path.node.computed) {
3498
- path = path.get("object");
3545
+ const idKey = String(addCapturedId(binding.identifier));
3546
+
3547
+ /**
3548
+ * Add the expression (potentially a memberexpr path) as a dependency.
3549
+ */
3550
+ let exprKey = idKey;
3551
+ if (dependency.isMemberExpression()) {
3552
+ let pathTokens = [];
3553
+ let current: NodePath<Expression> = dependency;
3554
+ while (current.isMemberExpression()) {
3555
+ const property = current.get("property") as NodePath<t.Identifier>;
3556
+ pathTokens.push(property.node.name);
3557
+ current = current.get("object");
3558
}
3559
3501
- path.skip();
3560
+ exprKey += "." + pathTokens.reverse().join(".");
3561
}
3562
3504
- // Store the top-level identifiers that are captured as well as the list
3505
- // of Places (including PropertyLoad)
3506
- let index: number;
3507
- if (!capturedIds.has(binding.identifier)) {
3508
- index = capturedIds.size;
3509
- capturedIds.set(binding.identifier, index);
3510
- } else {
3511
- index = capturedIds.get(binding.identifier)!;
3512
- }
3513
- let pathTokens = [];
3514
- let current = path;
3515
- while (current.isMemberExpression()) {
3516
- const property = path.get("property") as NodePath<t.Identifier>;
3517
- pathTokens.push(property.node.name);
3518
- current = current.get("object");
3519
- }
3520
- pathTokens.push(String(index));
3521
- pathTokens.reverse();
3522
- const pathKey = pathTokens.join(".");
3523
- if (!seenPaths.has(pathKey)) {
3524
- capturedRefs.add(lowerExpressionToTemporary(builder, path));
3525
- seenPaths.add(pathKey);
3563
+ if (!seenPaths.has(exprKey)) {
3564
+ let loweredDep: Place;
3565
+ if (dependency.isJSXIdentifier()) {
3566
+ loweredDep = lowerValueToTemporary(builder, {
3567
+ kind: "LoadLocal",
3568
+ place: lowerIdentifier(builder, dependency),
3569
+ loc: path.node.loc ?? GeneratedSource,
3570
+ });
3571
+ } else {
3572
+ loweredDep = lowerExpressionToTemporary(builder, dependency);
3573
+ }
3574
+ capturedRefs.add(loweredDep);
3575
+ seenPaths.add(exprKey);
3576
}
3577
}
3578
3579
fn.traverse({
3580
Expression(path) {
3531
- visit(path);
3581
+ if (path.isAssignmentExpression()) {
3582
+ // Babel has a bug where it doesn't visit the LHS of an
3583
+ // AssignmentExpression if it's an Identifier. Work around it by explicitly
3584
+ // visiting it.
3585
+ const left = path.get("left");
3586
+ if (left.isIdentifier()) {
3587
+ handleMaybeDependency(left);
3588
+ }
3589
+ return;
3590
+ } else if (path.isJSXElement()) {
3591
+ handleMaybeDependency(path.get("openingElement"));
3592
+ } else if (path.isMemberExpression() || path.isIdentifier()) {
3593
+ handleMaybeDependency(path);
3594
+ }
3595
},
3596
});
3597
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-ternary-value-in-jsx-attribute.expect.md
deleted
-27
@@ -1,27 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-import { RenderPropAsChild, StaticText1, StaticText2 } from "shared-runtime";
6
-
7
-function Component(props: { showText1: boolean }) {
8
- const Foo = props.showText1 ? StaticText1 : StaticText2;
9
-
10
- return <RenderPropAsChild items={[() => <Foo />]} />;
11
-}
12
-
13
-export const FIXTURE_ENTRYPOINT = {
14
- fn: Component,
15
- params: [{ showText1: false }],
16
-};
17
-
18
-```
19
-
20
-
21
-## Error
22
-
23
-```
24
-[ReactForget] Invariant: Expected value for identifier `28` to be initialized. (6:6)
25
-```
26
-
27
-
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/jsx-reactive-local-variable-member-expr.expect.md
new
+52
@@ -0,0 +1,52 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import * as sharedRuntime from "shared-runtime";
6
+
7
+function Component({
8
+ something,
9
+}: {
10
+ something: { StaticText1: React.ElementType };
11
+}) {
12
+ const Foo = something.StaticText1;
13
+ return () => <Foo />;
14
+}
15
+
16
+export const FIXTURE_ENTRYPOINT = {
17
+ fn: Component,
18
+ params: [{ something: sharedRuntime }],
19
+};
20
+
21
+```
22
+
23
+## Code
24
+
25
+```javascript
26
+import { unstable_useMemoCache as useMemoCache } from "react";
27
+import * as sharedRuntime from "shared-runtime";
28
+
29
+function Component(t13) {
30
+ const $ = useMemoCache(2);
31
+ const { something } = t13;
32
+
33
+ const Foo = something.StaticText1;
34
+ const c_0 = $[0] !== Foo;
35
+ let t0;
36
+ if (c_0) {
37
+ t0 = () => <Foo />;
38
+ $[0] = Foo;
39
+ $[1] = t0;
40
+ } else {
41
+ t0 = $[1];
42
+ }
43
+ return t0;
44
+}
45
+
46
+export const FIXTURE_ENTRYPOINT = {
47
+ fn: Component,
48
+ params: [{ something: sharedRuntime }],
49
+};
50
+
51
+```
52
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/jsx-reactive-local-variable-member-expr.tsx
new
+15
@@ -0,0 +1,15 @@
1
+import * as sharedRuntime from "shared-runtime";
2
+
3
+function Component({
4
+ something,
5
+}: {
6
+ something: { StaticText1: React.ElementType };
7
+}) {
8
+ const Foo = something.StaticText1;
9
+ return () => <Foo />;
10
+}
11
+
12
+export const FIXTURE_ENTRYPOINT = {
13
+ fn: Component,
14
+ params: [{ something: sharedRuntime }],
15
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/jsx-ternary-local-variable.expect.md
new
+65
@@ -0,0 +1,65 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { RenderPropAsChild, StaticText1, StaticText2 } from "shared-runtime";
6
+
7
+function Component(props: { showText1: boolean }) {
8
+ const Foo = props.showText1 ? StaticText1 : StaticText2;
9
+
10
+ return <RenderPropAsChild items={[() => <Foo key="0" />]} />;
11
+}
12
+
13
+export const FIXTURE_ENTRYPOINT = {
14
+ fn: Component,
15
+ params: [{ showText1: false }],
16
+};
17
+
18
+```
19
+
20
+## Code
21
+
22
+```javascript
23
+import { unstable_useMemoCache as useMemoCache } from "react";
24
+import { RenderPropAsChild, StaticText1, StaticText2 } from "shared-runtime";
25
+
26
+function Component(props) {
27
+ const $ = useMemoCache(6);
28
+ const Foo = props.showText1 ? StaticText1 : StaticText2;
29
+ const c_0 = $[0] !== Foo;
30
+ let t0;
31
+ if (c_0) {
32
+ t0 = () => <Foo key="0" />;
33
+ $[0] = Foo;
34
+ $[1] = t0;
35
+ } else {
36
+ t0 = $[1];
37
+ }
38
+ const c_2 = $[2] !== t0;
39
+ let t1;
40
+ if (c_2) {
41
+ t1 = [t0];
42
+ $[2] = t0;
43
+ $[3] = t1;
44
+ } else {
45
+ t1 = $[3];
46
+ }
47
+ const c_4 = $[4] !== t1;
48
+ let t2;
49
+ if (c_4) {
50
+ t2 = <RenderPropAsChild items={t1} />;
51
+ $[4] = t1;
52
+ $[5] = t2;
53
+ } else {
54
+ t2 = $[5];
55
+ }
56
+ return t2;
57
+}
58
+
59
+export const FIXTURE_ENTRYPOINT = {
60
+ fn: Component,
61
+ params: [{ showText1: false }],
62
+};
63
+
64
+```
65
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/jsx-ternary-local-variable.tsx
renamed
+1
-1
@@ -3,7 +3,7 @@ import { RenderPropAsChild, StaticText1, StaticText2 } from "shared-runtime";
3
function Component(props: { showText1: boolean }) {
4
const Foo = props.showText1 ? StaticText1 : StaticText2;
5
6
- return <RenderPropAsChild items={[() => <Foo />]} />;
6
+ return <RenderPropAsChild items={[() => <Foo key="0" />]} />;
7
}
8
9
export const FIXTURE_ENTRYPOINT = {