Fix for mutable ranges ended early with interleaving
Fixes T180504437. In MergeOverlappingReactiveScopes we track the active scopes and mark them as "ended" when reaching the first instruction after their mutable range. However, in cases of interleaving that will be merged, we could previously mark a scope as complete when it's original range was completed, even though the range would get extended post-merge. The fix here detects interleaving earlier, and eagerly updates the mutable ranges of the merged scopes to ensure that neither is "ended" earlier than it should. The repro here fails without this change.
Joe Savona committed
Mar 13, 2024 at 13:52 UTC
2940440dfb011173758e332da88ede7720b2509b
3 files changed
+155
-1
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/MergeOverlappingReactiveScopes.ts
+16
-1
@@ -229,8 +229,23 @@ class Context {
229
this.joinedScopes.union([current.shadowedBy, current.scope]);
230
}
231
} else if (found && current.shadowedBy === null) {
232
- // `scope` is shadowing `current`, but we don't know they are interleaved yet
232
+ // `scope` is shadowing `current` and may interleave
233
current.shadowedBy = scope;
234
+ if (current.scope.range.end > scope.range.end) {
235
+ /*
236
+ * Current is shadowed by `scope`, and we know that `current` will mutate
237
+ * again (per its range), so the scopes are already known to interleave.
238
+ *
239
+ * Eagerly extend the ranges of the scopes so that we don't prematurely end
240
+ * a scope relative to its eventual post-merge mutable range
241
+ */
242
+ const end = makeInstructionId(
243
+ Math.max(current.scope.range.end, scope.range.end)
244
+ );
245
+ current.scope.range.end = end;
246
+ scope.range.end = end;
247
+ this.joinedScopes.union([current.scope, scope]);
248
+ }
249
}
250
}
251
if (!currentBlock.seen.has(scope.id)) {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-instruction-part-of-already-closed-scope.expect.md
new
+116
@@ -0,0 +1,116 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @enableAssumeHooksFollowRulesOfReact
6
+import { Stringify, identity, useHook } from "shared-runtime";
7
+
8
+function Component({ index }) {
9
+ const data = useHook();
10
+
11
+ const a = identity(data, index);
12
+ const b = identity(data, index);
13
+ const c = identity(data, index);
14
+
15
+ return (
16
+ <div>
17
+ <Stringify value={identity(b)} />
18
+ <Stringify value={identity(a)} />
19
+ <Stringify value={identity(c)} />
20
+ </div>
21
+ );
22
+}
23
+
24
+export const FIXTURE_ENTRYPOINT = {
25
+ fn: Component,
26
+ params: [{ index: 0 }],
27
+};
28
+
29
+```
30
+
31
+## Code
32
+
33
+```javascript
34
+import { unstable_useMemoCache as useMemoCache } from "react"; // @enableAssumeHooksFollowRulesOfReact
35
+import { Stringify, identity, useHook } from "shared-runtime";
36
+
37
+function Component(t0) {
38
+ const $ = useMemoCache(17);
39
+ const { index } = t0;
40
+ const data = useHook();
41
+ let T0;
42
+ let t1;
43
+ let t2;
44
+ let t3;
45
+ if ($[0] !== data || $[1] !== index) {
46
+ const a = identity(data, index);
47
+ const b = identity(data, index);
48
+ const c = identity(data, index);
49
+
50
+ const t4 = identity(b);
51
+ if ($[6] !== t4) {
52
+ t2 = <Stringify value={t4} />;
53
+ $[6] = t4;
54
+ $[7] = t2;
55
+ } else {
56
+ t2 = $[7];
57
+ }
58
+ const t5 = identity(a);
59
+ if ($[8] !== t5) {
60
+ t3 = <Stringify value={t5} />;
61
+ $[8] = t5;
62
+ $[9] = t3;
63
+ } else {
64
+ t3 = $[9];
65
+ }
66
+ T0 = Stringify;
67
+ t1 = identity(c);
68
+ $[0] = data;
69
+ $[1] = index;
70
+ $[2] = T0;
71
+ $[3] = t1;
72
+ $[4] = t2;
73
+ $[5] = t3;
74
+ } else {
75
+ T0 = $[2];
76
+ t1 = $[3];
77
+ t2 = $[4];
78
+ t3 = $[5];
79
+ }
80
+ let t4;
81
+ if ($[10] !== T0 || $[11] !== t1) {
82
+ t4 = <T0 value={t1} />;
83
+ $[10] = T0;
84
+ $[11] = t1;
85
+ $[12] = t4;
86
+ } else {
87
+ t4 = $[12];
88
+ }
89
+ let t5;
90
+ if ($[13] !== t2 || $[14] !== t3 || $[15] !== t4) {
91
+ t5 = (
92
+ <div>
93
+ {t2}
94
+ {t3}
95
+ {t4}
96
+ </div>
97
+ );
98
+ $[13] = t2;
99
+ $[14] = t3;
100
+ $[15] = t4;
101
+ $[16] = t5;
102
+ } else {
103
+ t5 = $[16];
104
+ }
105
+ return t5;
106
+}
107
+
108
+export const FIXTURE_ENTRYPOINT = {
109
+ fn: Component,
110
+ params: [{ index: 0 }],
111
+};
112
+
113
+```
114
+
115
+### Eval output
116
+(kind: ok) <div><div>{"value":{"a":0,"b":"value1","c":true}}</div><div>{"value":{"a":0,"b":"value1","c":true}}</div><div>{"value":{"a":0,"b":"value1","c":true}}</div></div>
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-instruction-part-of-already-closed-scope.js
new
+23
@@ -0,0 +1,23 @@
1
+// @enableAssumeHooksFollowRulesOfReact
2
+import { Stringify, identity, useHook } from "shared-runtime";
3
+
4
+function Component({ index }) {
5
+ const data = useHook();
6
+
7
+ const a = identity(data, index);
8
+ const b = identity(data, index);
9
+ const c = identity(data, index);
10
+
11
+ return (
12
+ <div>
13
+ <Stringify value={identity(b)} />
14
+ <Stringify value={identity(a)} />
15
+ <Stringify value={identity(c)} />
16
+ </div>
17
+ );
18
+}
19
+
20
+export const FIXTURE_ENTRYPOINT = {
21
+ fn: Component,
22
+ params: [{ index: 0 }],
23
+};