Skip to content

Commit ee857f0

Browse files
committed
[compiler] Always error on async reassignments
Summary: Addresses the issue in #30109: any mutation of a local in an async function may occur after rendering has finished. ghstack-source-id: 5cf4b1db7ffff2db962d9b1fc3e9b37e10e3dff2 Pull Request resolved: #30111
1 parent ec23846 commit ee857f0

5 files changed

+54
-67
lines changed

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts

+17-3
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
*/
2020
export function validateLocalsNotReassignedAfterRender(fn: HIRFunction): void {
2121
const contextVariables = new Set<IdentifierId>();
22-
const reassignment = getContextReassignment(fn, contextVariables, false);
22+
const reassignment = getContextReassignment(fn, contextVariables, false, false);
2323
if (reassignment !== null) {
2424
CompilerError.throwInvalidReact({
2525
reason:
@@ -37,7 +37,8 @@ export function validateLocalsNotReassignedAfterRender(fn: HIRFunction): void {
3737
function getContextReassignment(
3838
fn: HIRFunction,
3939
contextVariables: Set<IdentifierId>,
40-
isFunctionExpression: boolean
40+
isFunctionExpression: boolean,
41+
isAsync: boolean,
4142
): Place | null {
4243
const reassigningFunctions = new Map<IdentifierId, Place>();
4344
for (const [, block] of fn.body.blocks) {
@@ -49,7 +50,8 @@ function getContextReassignment(
4950
let reassignment = getContextReassignment(
5051
value.loweredFunc.func,
5152
contextVariables,
52-
true
53+
true,
54+
isAsync || value.loweredFunc.func.async,
5355
);
5456
if (reassignment === null) {
5557
// If the function itself doesn't reassign, does one of its dependencies?
@@ -65,6 +67,18 @@ function getContextReassignment(
6567
}
6668
// if the function or its depends reassign, propagate that fact on the lvalue
6769
if (reassignment !== null) {
70+
if (isAsync || value.loweredFunc.func.async) {
71+
CompilerError.throwInvalidReact({
72+
reason:
73+
"Reassigning a variable in an async function can cause inconsistent behavior on subsequent renders. Consider using state instead",
74+
description:
75+
reassignment.identifier.name !== null &&
76+
reassignment.identifier.name.kind === "named"
77+
? `Variable \`${reassignment.identifier.name.value}\` cannot be reassigned after render`
78+
: "",
79+
loc: reassignment.loc,
80+
});
81+
}
6882
reassigningFunctions.set(lvalue.identifier.id, reassignment);
6983
}
7084
break;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
2+
## Input
3+
4+
```javascript
5+
function Component() {
6+
let value = null;
7+
const reassign = async () => {
8+
await foo().then((result) => {
9+
// Reassigning a local variable in an async function is *always* mutating
10+
// after render, so this should error regardless of where this ends up
11+
// getting called
12+
value = result;
13+
});
14+
};
15+
16+
const onClick = async () => {
17+
await reassign();
18+
};
19+
return <div onClick={onClick}>Click</div>;
20+
}
21+
22+
```
23+
24+
25+
## Error
26+
27+
```
28+
6 | // after render, so this should error regardless of where this ends up
29+
7 | // getting called
30+
> 8 | value = result;
31+
| ^^^^^ InvalidReact: Reassigning a variable in an async function can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `value` cannot be reassigned after render (8:8)
32+
9 | });
33+
10 | };
34+
11 |
35+
```
36+
37+

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-async-callback.expect.md

-58
This file was deleted.

compiler/packages/snap/src/SproutTodoFilter.ts

-6
Original file line numberDiff line numberDiff line change
@@ -483,12 +483,6 @@ const skipFilter = new Set([
483483
"rules-of-hooks/rules-of-hooks-93dc5d5e538a",
484484
"rules-of-hooks/rules-of-hooks-69521d94fa03",
485485

486-
// should error
487-
"todo.invalid-reassign-local-variable-in-jsx-callback",
488-
"todo.invalid-reassign-local-variable-in-hook-argument",
489-
"todo.invalid-reassign-local-variable-in-effect",
490-
"todo.invalid-reassign-local-variable-in-async-callback",
491-
492486
// bugs
493487
"bug-invalid-hoisting-functionexpr",
494488
"original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block",

0 commit comments

Comments
 (0)