Skip to content

Commit ec23846

Browse files
committed
[compiler] Repro for nested function local reassignment issue
Summary: Additional repro demonstrating a case that still exists after #30106 ghstack-source-id: 92b852068dd651cf52eb94b05a7b61e282ad25d5 Pull Request resolved: #30110
1 parent 00775d9 commit ec23846

File tree

3 files changed

+140
-0
lines changed

3 files changed

+140
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -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+
Original file line numberDiff line numberDiff line change
@@ -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+
}

Diff for: compiler/packages/snap/src/SproutTodoFilter.ts

+2
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,8 @@ const skipFilter = new Set([
506506

507507
// needs to be executed as a module
508508
"meta-property",
509+
510+
"todo.invalid-nested-function-reassign-local-variable-in-effect",
509511
]);
510512

511513
export default skipFilter;

0 commit comments

Comments
 (0)