[hir][patch] Fix small bug in duplicate scope merging logic
ghstack-source-id: 98966dd4c2b264329acad077427febc2a34a5edc Pull Request resolved: https://github.com/facebook/react-forget/pull/2906
Mofei Zhang committed
Apr 25, 2024 at 11:07 UTC
f196e1f7030a5ed4597d4095b6116cb08e6fdaa1
3 files changed
+114
-10
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopesHIR.ts
+12
-10
@@ -241,16 +241,6 @@ export function alignReactiveScopesToBlockScopesHIR(fn: HIRFunction): void {
241
const joinedScopes: DisjointSet<ReactiveScope> =
242
mergeOverlappingScopes(rootNode);
243
244
- joinedScopes.forEach((scope, groupScope) => {
245
- if (scope !== groupScope) {
246
- groupScope.range.start = makeInstructionId(
247
- Math.min(groupScope.range.start, scope.range.start)
248
- );
249
- groupScope.range.end = makeInstructionId(
250
- Math.max(groupScope.range.end, scope.range.end)
251
- );
252
- }
253
- });
244
/**
245
* Join scopes that begin and end at the same instructions
246
*/
@@ -269,6 +259,18 @@ export function alignReactiveScopesToBlockScopesHIR(fn: HIRFunction): void {
259
}
260
}
261
}
262
+
263
+ joinedScopes.forEach((scope, groupScope) => {
264
+ if (scope !== groupScope) {
265
+ groupScope.range.start = makeInstructionId(
266
+ Math.min(groupScope.range.start, scope.range.start)
267
+ );
268
+ groupScope.range.end = makeInstructionId(
269
+ Math.max(groupScope.range.end, scope.range.end)
270
+ );
271
+ }
272
+ });
273
+
274
for (const [place, originalScope] of placeScopes) {
275
const nextScope = joinedScopes.find(originalScope);
276
if (nextScope !== null && nextScope !== originalScope) {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/value-block-mutates-outer-value.expect.md
new
+76
@@ -0,0 +1,76 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { makeArray, useHook } from "shared-runtime";
6
+
7
+/**
8
+ * Here, the cond ? [...] : defaultList value block produces two
9
+ * new values (each with its own scope):
10
+ * $0 = ["text"]
11
+ * $1 = { text: $0 }
12
+ * The same value block also mutates customList, so it must be
13
+ * merged with the scope producing customList
14
+ */
15
+function Foo({ defaultList, cond }) {
16
+ const comparator = (a, b) => a - b;
17
+ useHook();
18
+ const customList = makeArray(1, 5, 2);
19
+ useHook();
20
+ const result = cond
21
+ ? [...customList.sort(comparator), { text: ["text"] }]
22
+ : defaultList;
23
+
24
+ return result;
25
+}
26
+
27
+export const FIXTURE_ENTRYPOINT = {
28
+ fn: Foo,
29
+ params: [{ defaultList: [2, 4], cond: true }],
30
+};
31
+
32
+```
33
+
34
+## Code
35
+
36
+```javascript
37
+import { unstable_useMemoCache as useMemoCache } from "react";
38
+import { makeArray, useHook } from "shared-runtime";
39
+
40
+/**
41
+ * Here, the cond ? [...] : defaultList value block produces two
42
+ * new values (each with its own scope):
43
+ * $0 = ["text"]
44
+ * $1 = { text: $0 }
45
+ * The same value block also mutates customList, so it must be
46
+ * merged with the scope producing customList
47
+ */
48
+function Foo(t0) {
49
+ const $ = useMemoCache(1);
50
+ const { defaultList, cond } = t0;
51
+ let t1;
52
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
53
+ t1 = (a, b) => a - b;
54
+ $[0] = t1;
55
+ } else {
56
+ t1 = $[0];
57
+ }
58
+ const comparator = t1;
59
+ useHook();
60
+ const customList = makeArray(1, 5, 2);
61
+ useHook();
62
+ const result = cond
63
+ ? [...customList.sort(comparator), { text: ["text"] }]
64
+ : defaultList;
65
+ return result;
66
+}
67
+
68
+export const FIXTURE_ENTRYPOINT = {
69
+ fn: Foo,
70
+ params: [{ defaultList: [2, 4], cond: true }],
71
+};
72
+
73
+```
74
+
75
+### Eval output
76
+(kind: ok) [1,2,5,{"text":["text"]}]
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/value-block-mutates-outer-value.ts
new
+26
@@ -0,0 +1,26 @@
1
+import { makeArray, useHook } from "shared-runtime";
2
+
3
+/**
4
+ * Here, the cond ? [...] : defaultList value block produces two
5
+ * new values (each with its own scope):
6
+ * $0 = ["text"]
7
+ * $1 = { text: $0 }
8
+ * The same value block also mutates customList, so it must be
9
+ * merged with the scope producing customList
10
+ */
11
+function Foo({ defaultList, cond }) {
12
+ const comparator = (a, b) => a - b;
13
+ useHook();
14
+ const customList = makeArray(1, 5, 2);
15
+ useHook();
16
+ const result = cond
17
+ ? [...customList.sort(comparator), { text: ["text"] }]
18
+ : defaultList;
19
+
20
+ return result;
21
+}
22
+
23
+export const FIXTURE_ENTRYPOINT = {
24
+ fn: Foo,
25
+ params: [{ defaultList: [2, 4], cond: true }],
26
+};