Fix duplicate declaration from MergeConsecutiveScopes
This is a distilled version of the duplicate declaration @mofeiZ and I saw when trying to sync latest Forget internally, plus a fix to avoid the duplicate instruction.
Joe Savona committed
Oct 2, 2023 at 16:40 UTC
41cb3c722ee764ba5eeb4b4bf4556983cd5c57db
3 files changed
+95
-1
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/MergeConsecutiveScopes.ts
+10
-1
@@ -108,6 +108,9 @@ class Transform extends ReactiveFunctionTransform<void> {
108
// The updated set of instructions for the block. Stays null until
109
// we make changes (ie merge scopes)
110
let nextInstructions: ReactiveBlock | null = null;
111
+ // The maximum index within the original instructions that we have reached.
112
+ // Used to avoid emitting duplicate instructions
113
+ let maxIndex: number = 0;
114
115
// Called when we find some instruction that cannot be merged into a
116
// preceding scope, or we otherwise need to reset and not consider
@@ -121,8 +124,14 @@ class Transform extends ReactiveFunctionTransform<void> {
124
if (currentScope !== null) {
125
nextInstructions.push(...block.slice(currentScope.to, index));
126
}
124
- if (index < block.length) {
127
+ // We can sometimes call resetCurrentScope twice for the same index,
128
+ // such as when an instruction resets and then a subsequent scope also resets.
129
+ // This is the only case in which we push instructions w/o gating on
130
+ // `currentScope != null`, so we avoid duplicates by checking the max index
131
+ // already emitted.
132
+ if (index < block.length && index > maxIndex) {
133
nextInstructions.push(block[index]!);
134
+ maxIndex = index;
135
}
136
}
137
currentScope = null;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-duplicate-instruction-from-merge-consecutive-scopes.expect.md
new
+69
@@ -0,0 +1,69 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @enableMergeConsecutiveScopes
6
+function Component(id) {
7
+ const bar = (() => {})();
8
+
9
+ return (
10
+ <>
11
+ <Bar title={bar} />
12
+ <Bar title={id ? true : false} />
13
+ </>
14
+ );
15
+}
16
+
17
+export const FIXTURE_ENTRYPOINT = {
18
+ fn: Component,
19
+ params: [null],
20
+};
21
+
22
+```
23
+
24
+## Code
25
+
26
+```javascript
27
+import { unstable_useMemoCache as useMemoCache } from "react"; // @enableMergeConsecutiveScopes
28
+function Component(id) {
29
+ const $ = useMemoCache(4);
30
+ let t0;
31
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
32
+ t0 = (() => {})();
33
+ $[0] = t0;
34
+ } else {
35
+ t0 = $[0];
36
+ }
37
+ const bar = t0;
38
+ let t1;
39
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
40
+ t1 = <Bar title={bar} />;
41
+ $[1] = t1;
42
+ } else {
43
+ t1 = $[1];
44
+ }
45
+ const t2 = id ? true : false;
46
+ const c_2 = $[2] !== t2;
47
+ let t3;
48
+ if (c_2) {
49
+ t3 = (
50
+ <>
51
+ {t1}
52
+ <Bar title={t2} />
53
+ </>
54
+ );
55
+ $[2] = t2;
56
+ $[3] = t3;
57
+ } else {
58
+ t3 = $[3];
59
+ }
60
+ return t3;
61
+}
62
+
63
+export const FIXTURE_ENTRYPOINT = {
64
+ fn: Component,
65
+ params: [null],
66
+};
67
+
68
+```
69
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-duplicate-instruction-from-merge-consecutive-scopes.js
new
+16
@@ -0,0 +1,16 @@
1
+// @enableMergeConsecutiveScopes
2
+function Component(id) {
3
+ const bar = (() => {})();
4
+
5
+ return (
6
+ <>
7
+ <Bar title={bar} />
8
+ <Bar title={id ? true : false} />
9
+ </>
10
+ );
11
+}
12
+
13
+export const FIXTURE_ENTRYPOINT = {
14
+ fn: Component,
15
+ params: [null],
16
+};