Fix promotion of locals referenced outside of scopes
Joe Savona committed
Mar 6, 2024 at 16:19 UTC
449aa70f99c4984e7550fb9ef1aa16b1b60d6544
3 files changed
+75
-5
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+8
-5
@@ -23,6 +23,7 @@ import {
23
ReactiveScopeDependency,
24
ReactiveTerminalStatement,
25
ReactiveValue,
26
+ ScopeId,
27
} from "../HIR/HIR";
28
import {
29
eachInstructionValueOperand,
@@ -75,18 +76,18 @@ type TemporariesUsedOutsideDefiningScope = {
76
* tracks all relevant temporary declarations (currently LoadLocal and PropertyLoad)
77
* and the scope where they are defined
78
*/
78
- declarations: Map<IdentifierId, ReactiveScope>;
79
+ declarations: Map<IdentifierId, ScopeId>;
80
// temporaries used outside of their defining scope
81
usedOutsideDeclaringScope: Set<IdentifierId>;
82
};
83
class FindPromotedTemporaries extends ReactiveFunctionVisitor<TemporariesUsedOutsideDefiningScope> {
83
- scopes: Array<ReactiveScope> = [];
84
+ scopes: Array<ScopeId> = [];
85
86
override visitScope(
87
scope: ReactiveScopeBlock,
88
state: TemporariesUsedOutsideDefiningScope
89
): void {
89
- this.scopes.push(scope.scope);
90
+ this.scopes.push(scope.scope.id);
91
this.traverseScope(scope, state);
92
this.scopes.pop();
93
}
@@ -95,6 +96,9 @@ class FindPromotedTemporaries extends ReactiveFunctionVisitor<TemporariesUsedOut
96
instruction: ReactiveInstruction,
97
state: TemporariesUsedOutsideDefiningScope
98
): void {
99
+ // Visit all places first, then record temporaries which may need to be promoted
100
+ this.traverseInstruction(instruction, state);
101
+
102
const scope = this.scopes.at(-1);
103
if (instruction.lvalue === null || scope === undefined) {
104
return;
@@ -110,7 +114,6 @@ class FindPromotedTemporaries extends ReactiveFunctionVisitor<TemporariesUsedOut
114
break;
115
}
116
}
113
- this.traverseInstruction(instruction, state);
117
}
118
119
override visitPlace(
@@ -119,7 +122,7 @@ class FindPromotedTemporaries extends ReactiveFunctionVisitor<TemporariesUsedOut
122
state: TemporariesUsedOutsideDefiningScope
123
): void {
124
const declaringScope = state.declarations.get(place.identifier.id);
122
- if (this.scopes.length === 0 || declaringScope === undefined) {
125
+ if (declaringScope === undefined) {
126
return;
127
}
128
if (this.scopes.indexOf(declaringScope) === -1) {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-no-value-for-temporary.expect.md
new
+60
@@ -0,0 +1,60 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @flow @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions
6
+function Component(listItem, thread) {
7
+ const isFoo = isFooThread(thread.threadType);
8
+ const body = useBar(listItem, [getBadgeText(listItem, isFoo)]);
9
+
10
+ return body;
11
+}
12
+
13
+```
14
+
15
+## Code
16
+
17
+```javascript
18
+import { unstable_useMemoCache as useMemoCache } from "react";
19
+function Component(listItem, thread) {
20
+ const $ = useMemoCache(7);
21
+ let t0;
22
+ let t1;
23
+ let t2;
24
+ if ($[0] !== thread.threadType || $[1] !== listItem) {
25
+ const isFoo = isFooThread(thread.threadType);
26
+ t1 = useBar;
27
+ t2 = listItem;
28
+ t0 = getBadgeText(listItem, isFoo);
29
+ $[0] = thread.threadType;
30
+ $[1] = listItem;
31
+ $[2] = t0;
32
+ $[3] = t1;
33
+ $[4] = t2;
34
+ } else {
35
+ t0 = $[2];
36
+ t1 = $[3];
37
+ t2 = $[4];
38
+ }
39
+ let t3;
40
+ if ($[5] !== t0) {
41
+ t3 = [t0];
42
+ $[5] = t0;
43
+ $[6] = t3;
44
+ } else {
45
+ t3 = $[6];
46
+ }
47
+ const body = t1(t2, t3);
48
+ return body;
49
+}
50
+
51
+```
52
+
53
+### Eval output
54
+(kind: exception) Fixture not implemented!
55
+logs: ['The above error occurred in the <WrapperTestComponent> component:\n' +
56
+ '\n' +
57
+ ' at WrapperTestComponent (<project_root>/packages/snap/dist/sprout/evaluator.js:54:26)\n' +
58
+ '\n' +
59
+ 'Consider adding an error boundary to your tree to customize error handling behavior.\n' +
60
+ 'Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries.']
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-no-value-for-temporary.js
new
+7
@@ -0,0 +1,7 @@
1
+// @flow @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions
2
+function Component(listItem, thread) {
3
+ const isFoo = isFooThread(thread.threadType);
4
+ const body = useBar(listItem, [getBadgeText(listItem, isFoo)]);
5
+
6
+ return body;
7
+}