[compiler] Repro for nested function local reassignment issue
Summary: Additional repro demonstrating a case that still exists after #30106 ghstack-source-id: f03176fd88b29098d0263f86c1a7126e214997e5 Pull Request resolved: https://github.com/facebook/react/pull/30110
Mike Vitousek committed
Jun 26, 2024 at 17:07 UTC
1f59d07042466fa958d74062592cc31890e88661
3 files changed
+140
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-nested-function-reassign-local-variable-in-effect.expect.md
new
+101
@@ -0,0 +1,101 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { useEffect } from "react";
6
+function Component() {
7
+ let local;
8
+ const mk_reassignlocal = () => {
9
+ // Create the reassignment function inside another function, then return it
10
+ const reassignLocal = (newValue) => {
11
+ local = newValue;
12
+ };
13
+ return reassignLocal;
14
+ };
15
+ const reassignLocal = mk_reassignlocal();
16
+ const onMount = (newValue) => {
17
+ reassignLocal("hello");
18
+ if (local === newValue) {
19
+ // Without React Compiler, `reassignLocal` is freshly created
20
+ // on each render, capturing a binding to the latest `local`,
21
+ // such that invoking reassignLocal will reassign the same
22
+ // binding that we are observing in the if condition, and
23
+ // we reach this branch
24
+ console.log("`local` was updated!");
25
+ } else {
26
+ // With React Compiler enabled, `reassignLocal` is only created
27
+ // once, capturing a binding to `local` in that render pass.
28
+ // Therefore, calling `reassignLocal` will reassign the wrong
29
+ // version of `local`, and not update the binding we are checking
30
+ // in the if condition.
31
+ //
32
+ // To protect against this, we disallow reassigning locals from
33
+ // functions that escape
34
+ throw new Error("`local` not updated!");
35
+ }
36
+ };
37
+ useEffect(() => {
38
+ onMount();
39
+ }, [onMount]);
40
+ return "ok";
41
+}
42
+
43
+```
44
+
45
+## Code
46
+
47
+```javascript
48
+import { c as _c } from "react/compiler-runtime";
49
+import { useEffect } from "react";
50
+function Component() {
51
+ const $ = _c(4);
52
+ let local;
53
+ let t0;
54
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
55
+ const mk_reassignlocal = () => {
56
+ const reassignLocal = (newValue) => {
57
+ local = newValue;
58
+ };
59
+ return reassignLocal;
60
+ };
61
+
62
+ t0 = mk_reassignlocal();
63
+ $[0] = t0;
64
+ } else {
65
+ t0 = $[0];
66
+ }
67
+ const reassignLocal_0 = t0;
68
+ let t1;
69
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
70
+ t1 = (newValue_0) => {
71
+ reassignLocal_0("hello");
72
+ if (local === newValue_0) {
73
+ console.log("`local` was updated!");
74
+ } else {
75
+ throw new Error("`local` not updated!");
76
+ }
77
+ };
78
+ $[1] = t1;
79
+ } else {
80
+ t1 = $[1];
81
+ }
82
+ const onMount = t1;
83
+ let t2;
84
+ let t3;
85
+ if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
86
+ t2 = () => {
87
+ onMount();
88
+ };
89
+ t3 = [onMount];
90
+ $[2] = t2;
91
+ $[3] = t3;
92
+ } else {
93
+ t2 = $[2];
94
+ t3 = $[3];
95
+ }
96
+ useEffect(t2, t3);
97
+ return "ok";
98
+}
99
+
100
+```
101
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-nested-function-reassign-local-variable-in-effect.js
new
+37
@@ -0,0 +1,37 @@
1
+import { useEffect } from "react";
2
+function Component() {
3
+ let local;
4
+ const mk_reassignlocal = () => {
5
+ // Create the reassignment function inside another function, then return it
6
+ const reassignLocal = (newValue) => {
7
+ local = newValue;
8
+ };
9
+ return reassignLocal;
10
+ };
11
+ const reassignLocal = mk_reassignlocal();
12
+ const onMount = (newValue) => {
13
+ reassignLocal("hello");
14
+ if (local === newValue) {
15
+ // Without React Compiler, `reassignLocal` is freshly created
16
+ // on each render, capturing a binding to the latest `local`,
17
+ // such that invoking reassignLocal will reassign the same
18
+ // binding that we are observing in the if condition, and
19
+ // we reach this branch
20
+ console.log("`local` was updated!");
21
+ } else {
22
+ // With React Compiler enabled, `reassignLocal` is only created
23
+ // once, capturing a binding to `local` in that render pass.
24
+ // Therefore, calling `reassignLocal` will reassign the wrong
25
+ // version of `local`, and not update the binding we are checking
26
+ // in the if condition.
27
+ //
28
+ // To protect against this, we disallow reassigning locals from
29
+ // functions that escape
30
+ throw new Error("`local` not updated!");
31
+ }
32
+ };
33
+ useEffect(() => {
34
+ onMount();
35
+ }, [onMount]);
36
+ return "ok";
37
+}
compiler/packages/snap/src/SproutTodoFilter.ts
+2
@@ -506,6 +506,8 @@ const skipFilter = new Set([
506
507
// needs to be executed as a module
508
"meta-property",
509
+
510
+ "todo.invalid-nested-function-reassign-local-variable-in-effect",
511
]);
512
513
export default skipFilter;