Repro function expr hoisting (#29615)
Modified version of @mofeiZ's #29232 with CI passing (had to run prettier) --------- Co-authored-by: Mofei Zhang <feifei0@meta.com>
Joseph Savona committed
May 28, 2024 at 10:06 UTC
4ec6a6f71475a6f2fee39a0e604ddbbd2f124164
6 files changed
+291
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.expect.md
new
+93
@@ -0,0 +1,93 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { Stringify } from "shared-runtime";
6
+
7
+/**
8
+ * We currently hoist the accessed properties of function expressions,
9
+ * regardless of control flow. This is simply because we wrote support for
10
+ * function expressions before doing a lot of work in PropagateScopeDeps
11
+ * to handle conditionally accessed dependencies.
12
+ *
13
+ * Current evaluator error:
14
+ * Found differences in evaluator results
15
+ * Non-forget (expected):
16
+ * (kind: ok) <div>{"shouldInvokeFns":true,"callback":{"kind":"Function","result":null}}</div>
17
+ * Forget:
18
+ * (kind: exception) Cannot read properties of null (reading 'prop')
19
+ */
20
+function Component({ obj, isObjNull }) {
21
+ const callback = () => {
22
+ if (!isObjNull) {
23
+ return obj.prop;
24
+ } else {
25
+ return null;
26
+ }
27
+ };
28
+ return <Stringify shouldInvokeFns={true} callback={callback} />;
29
+}
30
+
31
+export const FIXTURE_ENTRYPOINT = {
32
+ fn: Component,
33
+ params: [{ obj: null, isObjNull: true }],
34
+};
35
+
36
+```
37
+
38
+## Code
39
+
40
+```javascript
41
+import { c as _c } from "react/compiler-runtime";
42
+import { Stringify } from "shared-runtime";
43
+
44
+/**
45
+ * We currently hoist the accessed properties of function expressions,
46
+ * regardless of control flow. This is simply because we wrote support for
47
+ * function expressions before doing a lot of work in PropagateScopeDeps
48
+ * to handle conditionally accessed dependencies.
49
+ *
50
+ * Current evaluator error:
51
+ * Found differences in evaluator results
52
+ * Non-forget (expected):
53
+ * (kind: ok) <div>{"shouldInvokeFns":true,"callback":{"kind":"Function","result":null}}</div>
54
+ * Forget:
55
+ * (kind: exception) Cannot read properties of null (reading 'prop')
56
+ */
57
+function Component(t0) {
58
+ const $ = _c(5);
59
+ const { obj, isObjNull } = t0;
60
+ let t1;
61
+ if ($[0] !== isObjNull || $[1] !== obj.prop) {
62
+ t1 = () => {
63
+ if (!isObjNull) {
64
+ return obj.prop;
65
+ } else {
66
+ return null;
67
+ }
68
+ };
69
+ $[0] = isObjNull;
70
+ $[1] = obj.prop;
71
+ $[2] = t1;
72
+ } else {
73
+ t1 = $[2];
74
+ }
75
+ const callback = t1;
76
+ let t2;
77
+ if ($[3] !== callback) {
78
+ t2 = <Stringify shouldInvokeFns={true} callback={callback} />;
79
+ $[3] = callback;
80
+ $[4] = t2;
81
+ } else {
82
+ t2 = $[4];
83
+ }
84
+ return t2;
85
+}
86
+
87
+export const FIXTURE_ENTRYPOINT = {
88
+ fn: Component,
89
+ params: [{ obj: null, isObjNull: true }],
90
+};
91
+
92
+```
93
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.tsx
new
+30
@@ -0,0 +1,30 @@
1
+import { Stringify } from "shared-runtime";
2
+
3
+/**
4
+ * We currently hoist the accessed properties of function expressions,
5
+ * regardless of control flow. This is simply because we wrote support for
6
+ * function expressions before doing a lot of work in PropagateScopeDeps
7
+ * to handle conditionally accessed dependencies.
8
+ *
9
+ * Current evaluator error:
10
+ * Found differences in evaluator results
11
+ * Non-forget (expected):
12
+ * (kind: ok) <div>{"shouldInvokeFns":true,"callback":{"kind":"Function","result":null}}</div>
13
+ * Forget:
14
+ * (kind: exception) Cannot read properties of null (reading 'prop')
15
+ */
16
+function Component({ obj, isObjNull }) {
17
+ const callback = () => {
18
+ if (!isObjNull) {
19
+ return obj.prop;
20
+ } else {
21
+ return null;
22
+ }
23
+ };
24
+ return <Stringify shouldInvokeFns={true} callback={callback} />;
25
+}
26
+
27
+export const FIXTURE_ENTRYPOINT = {
28
+ fn: Component,
29
+ params: [{ obj: null, isObjNull: true }],
30
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md
new
+119
@@ -0,0 +1,119 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import invariant from "invariant";
6
+import {
7
+ makeObject_Primitives,
8
+ mutate,
9
+ sum,
10
+ useIdentity,
11
+} from "shared-runtime";
12
+
13
+/**
14
+ * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening
15
+ * those scopes. Here, `z`'s original memo block is removed due to the inner hook call.
16
+ * However, we also infer that `z` is non-reactive and does not need to be a memo
17
+ * dependency.
18
+ *
19
+ * Current evaluator error:
20
+ * Found differences in evaluator results
21
+ * Non-forget (expected):
22
+ * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
23
+ * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
24
+ * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
25
+ * Forget:
26
+ * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
27
+ * [[ (exception in render) Invariant Violation: oh no! ]]
28
+ * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
29
+ */
30
+
31
+function MyApp({ count }) {
32
+ const z = makeObject_Primitives();
33
+ const x = useIdentity(2);
34
+ const y = sum(x, count);
35
+ mutate(z);
36
+ const thing = [y, z];
37
+ if (thing[1] !== z) {
38
+ invariant(false, "oh no!");
39
+ }
40
+ return thing;
41
+}
42
+
43
+export const FIXTURE_ENTRYPOINT = {
44
+ fn: MyApp,
45
+ params: [{ count: 2 }],
46
+ sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }],
47
+};
48
+
49
+```
50
+
51
+## Code
52
+
53
+```javascript
54
+import { c as _c } from "react/compiler-runtime";
55
+import invariant from "invariant";
56
+import {
57
+ makeObject_Primitives,
58
+ mutate,
59
+ sum,
60
+ useIdentity,
61
+} from "shared-runtime";
62
+
63
+/**
64
+ * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening
65
+ * those scopes. Here, `z`'s original memo block is removed due to the inner hook call.
66
+ * However, we also infer that `z` is non-reactive and does not need to be a memo
67
+ * dependency.
68
+ *
69
+ * Current evaluator error:
70
+ * Found differences in evaluator results
71
+ * Non-forget (expected):
72
+ * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
73
+ * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
74
+ * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
75
+ * Forget:
76
+ * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
77
+ * [[ (exception in render) Invariant Violation: oh no! ]]
78
+ * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
79
+ */
80
+
81
+function MyApp(t0) {
82
+ const $ = _c(5);
83
+ const { count } = t0;
84
+ const z = makeObject_Primitives();
85
+ const x = useIdentity(2);
86
+ let t1;
87
+ if ($[0] !== x || $[1] !== count) {
88
+ t1 = sum(x, count);
89
+ $[0] = x;
90
+ $[1] = count;
91
+ $[2] = t1;
92
+ } else {
93
+ t1 = $[2];
94
+ }
95
+ const y = t1;
96
+ mutate(z);
97
+ let t2;
98
+ if ($[3] !== y) {
99
+ t2 = [y, z];
100
+ $[3] = y;
101
+ $[4] = t2;
102
+ } else {
103
+ t2 = $[4];
104
+ }
105
+ const thing = t2;
106
+ if (thing[1] !== z) {
107
+ invariant(false, "oh no!");
108
+ }
109
+ return thing;
110
+}
111
+
112
+export const FIXTURE_ENTRYPOINT = {
113
+ fn: MyApp,
114
+ params: [{ count: 2 }],
115
+ sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }],
116
+};
117
+
118
+```
119
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts
new
+43
@@ -0,0 +1,43 @@
1
+import invariant from "invariant";
2
+import {
3
+ makeObject_Primitives,
4
+ mutate,
5
+ sum,
6
+ useIdentity,
7
+} from "shared-runtime";
8
+
9
+/**
10
+ * Exposes fundamental issue with pruning 'non-reactive' dependencies + flattening
11
+ * those scopes. Here, `z`'s original memo block is removed due to the inner hook call.
12
+ * However, we also infer that `z` is non-reactive and does not need to be a memo
13
+ * dependency.
14
+ *
15
+ * Current evaluator error:
16
+ * Found differences in evaluator results
17
+ * Non-forget (expected):
18
+ * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
19
+ * [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
20
+ * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
21
+ * Forget:
22
+ * (kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
23
+ * [[ (exception in render) Invariant Violation: oh no! ]]
24
+ * [5,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
25
+ */
26
+
27
+function MyApp({ count }) {
28
+ const z = makeObject_Primitives();
29
+ const x = useIdentity(2);
30
+ const y = sum(x, count);
31
+ mutate(z);
32
+ const thing = [y, z];
33
+ if (thing[1] !== z) {
34
+ invariant(false, "oh no!");
35
+ }
36
+ return thing;
37
+}
38
+
39
+export const FIXTURE_ENTRYPOINT = {
40
+ fn: MyApp,
41
+ params: [{ count: 2 }],
42
+ sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }],
43
+};
compiler/packages/snap/src/SproutTodoFilter.ts
+2
@@ -486,6 +486,8 @@ const skipFilter = new Set([
486
487
// bugs
488
"bug-invalid-reactivity-value-block",
489
+ "bug-invalid-pruned-scope-leaks-value",
490
+ "bug-invalid-hoisting-functionexpr",
491
"original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block",
492
"original-reactive-scopes-fork/bug-hoisted-declaration-with-scope",
493
compiler/packages/snap/src/sprout/shared-runtime.ts
+4
@@ -176,6 +176,10 @@ export function useNoAlias(...args: Array<any>): object {
176
return noAliasObject;
177
}
178
179
+export function useIdentity<T>(arg: T): T {
180
+ return arg;
181
+}
182
+
183
export function invoke<T extends Array<any>, ReturnType>(
184
fn: (...input: T) => ReturnType,
185
...params: T