Skip to content

Commit fcfbfc1

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: 9f15cf0f144c0badd6009ceb51df43a50399d82b Pull Request resolved: #30111
1 parent 1f59d07 commit fcfbfc1

5 files changed

+59
-67
lines changed

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

+22-3
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@ 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(
23+
fn,
24+
contextVariables,
25+
false,
26+
false
27+
);
2328
if (reassignment !== null) {
2429
CompilerError.throwInvalidReact({
2530
reason:
@@ -37,7 +42,8 @@ export function validateLocalsNotReassignedAfterRender(fn: HIRFunction): void {
3742
function getContextReassignment(
3843
fn: HIRFunction,
3944
contextVariables: Set<IdentifierId>,
40-
isFunctionExpression: boolean
45+
isFunctionExpression: boolean,
46+
isAsync: boolean
4147
): Place | null {
4248
const reassigningFunctions = new Map<IdentifierId, Place>();
4349
for (const [, block] of fn.body.blocks) {
@@ -49,7 +55,8 @@ function getContextReassignment(
4955
let reassignment = getContextReassignment(
5056
value.loweredFunc.func,
5157
contextVariables,
52-
true
58+
true,
59+
isAsync || value.loweredFunc.func.async
5360
);
5461
if (reassignment === null) {
5562
// If the function itself doesn't reassign, does one of its dependencies?
@@ -65,6 +72,18 @@ function getContextReassignment(
6572
}
6673
// if the function or its depends reassign, propagate that fact on the lvalue
6774
if (reassignment !== null) {
75+
if (isAsync || value.loweredFunc.func.async) {
76+
CompilerError.throwInvalidReact({
77+
reason:
78+
"Reassigning a variable in an async function can cause inconsistent behavior on subsequent renders. Consider using state instead",
79+
description:
80+
reassignment.identifier.name !== null &&
81+
reassignment.identifier.name.kind === "named"
82+
? `Variable \`${reassignment.identifier.name.value}\` cannot be reassigned after render`
83+
: "",
84+
loc: reassignment.loc,
85+
});
86+
}
6887
reassigningFunctions.set(lvalue.identifier.id, reassignment);
6988
}
7089
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)