Fix capturedRefs collection for lambdas
When we calculate the dependencies of a FunctionExpression we were only adding new items if the binding identifier had not been seen yet. That is correct for `capturedIds` since its the set of identifiers, but incorrect for `capturedRefs` since its an array of all the distinct places. This meant that if a function expression referenced multiple properties of the same binding, we'd only record the first one. We now correctly record all of them.
Joe Savona committed
May 9, 2023 at 10:38 UTC
50c3bd9d98906b8544a23e6efda31b235071cfb9
6 files changed
+42
-22
compiler/forget/src/HIR/BuildHIR.ts
+23
-3
@@ -2753,8 +2753,9 @@ function gatherCapturedDeps(
2753
fn: NodePath<t.FunctionExpression | t.ArrowFunctionExpression>,
2754
componentScope: Scope
2755
): { identifiers: t.Identifier[]; refs: Place[] } {
2756
- const capturedIds: Set<t.Identifier> = new Set();
2756
+ const capturedIds: Map<t.Identifier, number> = new Map();
2757
const capturedRefs: Set<Place> = new Set();
2758
+ const seenPaths: Set<string> = new Set();
2759
2760
// Capture all the scopes from the parent of this function up to and including
2761
// the component scope.
@@ -2807,9 +2808,28 @@ function gatherCapturedDeps(
2808
path.skip();
2809
}
2810
2811
+ // Store the top-level identifiers that are captured as well as the list
2812
+ // of Places (including PropertyLoad)
2813
+ let index: number;
2814
if (!capturedIds.has(binding.identifier)) {
2811
- capturedIds.add(binding.identifier);
2815
+ index = capturedIds.size;
2816
+ capturedIds.set(binding.identifier, index);
2817
+ } else {
2818
+ index = capturedIds.get(binding.identifier)!;
2819
+ }
2820
+ let pathTokens = [];
2821
+ let current = path;
2822
+ while (current.isMemberExpression()) {
2823
+ const property = path.get("property") as NodePath<t.Identifier>;
2824
+ pathTokens.push(property.node.name);
2825
+ current = current.get("object");
2826
+ }
2827
+ pathTokens.push(String(index));
2828
+ pathTokens.reverse();
2829
+ const pathKey = pathTokens.join(".");
2830
+ if (!seenPaths.has(pathKey)) {
2831
capturedRefs.add(lowerExpressionToTemporary(builder, path));
2832
+ seenPaths.add(pathKey);
2833
}
2834
}
2835
@@ -2819,7 +2839,7 @@ function gatherCapturedDeps(
2839
},
2840
});
2841
2822
- return { identifiers: [...capturedIds], refs: [...capturedRefs] };
2842
+ return { identifiers: [...capturedIds.keys()], refs: [...capturedRefs] };
2843
}
2844
2845
function notNull<T>(value: T | null): value is T {
compiler/forget/src/__tests__/fixtures/compiler/useMemo-if-else-multiple-return.expect.md
+4
-4
@@ -20,7 +20,7 @@ function Component(props) {
20
import { unstable_useMemoCache as useMemoCache } from "react";
21
function Component(props) {
22
const $ = useMemoCache(4);
23
- let t17 = undefined;
23
+ let t21 = undefined;
24
if (props.cond) {
25
const c_0 = $[0] !== props.a;
26
let t0;
@@ -31,7 +31,7 @@ function Component(props) {
31
} else {
32
t0 = $[1];
33
}
34
- t17 = t0;
34
+ t21 = t0;
35
} else {
36
const c_2 = $[2] !== props.b;
37
let t1;
@@ -42,9 +42,9 @@ function Component(props) {
42
} else {
43
t1 = $[3];
44
}
45
- t17 = t1;
45
+ t21 = t1;
46
}
47
- const x = t17;
47
+ const x = t21;
48
return x;
49
}
50
compiler/forget/src/__tests__/fixtures/compiler/useMemo-independently-memoizeable.expect.md
+2
-2
@@ -51,8 +51,8 @@ function Component(props) {
51
} else {
52
t2 = $[6];
53
}
54
- const t25 = t2;
55
- const [a_0, b_0] = t25;
54
+ const t27 = t2;
55
+ const [a_0, b_0] = t27;
56
const c_7 = $[7] !== a_0;
57
const c_8 = $[8] !== b_0;
58
let t3;
compiler/forget/src/__tests__/fixtures/compiler/useMemo-logical.expect.md
+2
-2
@@ -13,8 +13,8 @@ function Component(props) {
13
14
```javascript
15
function Component(props) {
16
- const t15 = props.a && props.b;
17
- const x = t15;
16
+ const t17 = props.a && props.b;
17
+ const x = t17;
18
return x;
19
}
20
compiler/forget/src/__tests__/fixtures/compiler/useMemo-multiple-if-else.expect.md
+7
-7
@@ -26,25 +26,25 @@ import { unstable_useMemoCache as useMemoCache } from "react";
26
function Component(props) {
27
const $ = useMemoCache(2);
28
const c_0 = $[0] !== props;
29
- let t26;
29
+ let t32;
30
if (c_0) {
31
const y = [];
32
if (props.cond) {
33
y.push(props.a);
34
}
35
- t26 = undefined;
35
+ t32 = undefined;
36
if (props.cond2) {
37
- t26 = y;
37
+ t32 = y;
38
} else {
39
y.push(props.b);
40
- t26 = y;
40
+ t32 = y;
41
}
42
$[0] = props;
43
- $[1] = t26;
43
+ $[1] = t32;
44
} else {
45
- t26 = $[1];
45
+ t32 = $[1];
46
}
47
- const x = t26;
47
+ const x = t32;
48
return x;
49
}
50
compiler/forget/src/__tests__/fixtures/compiler/useMemo-switch-no-fallthrough.expect.md
+4
-4
@@ -22,17 +22,17 @@ function Component(props) {
22
23
```javascript
24
function Component(props) {
25
- let t14 = undefined;
25
+ let t18 = undefined;
26
bb8: switch (props.key) {
27
case "key": {
28
- t14 = props.value;
28
+ t18 = props.value;
29
break bb8;
30
}
31
default: {
32
- t14 = props.defaultValue;
32
+ t18 = props.defaultValue;
33
}
34
}
35
- const x = t14;
35
+ const x = t18;
36
return x;
37
}
38