diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts index e83e5e69b5d21..f36f7adfd901f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts @@ -19,7 +19,12 @@ import { */ export function validateLocalsNotReassignedAfterRender(fn: HIRFunction): void { const contextVariables = new Set(); - const reassignment = getContextReassignment(fn, contextVariables, false); + const reassignment = getContextReassignment( + fn, + contextVariables, + false, + false + ); if (reassignment !== null) { CompilerError.throwInvalidReact({ reason: @@ -37,7 +42,8 @@ export function validateLocalsNotReassignedAfterRender(fn: HIRFunction): void { function getContextReassignment( fn: HIRFunction, contextVariables: Set, - isFunctionExpression: boolean + isFunctionExpression: boolean, + isAsync: boolean ): Place | null { const reassigningFunctions = new Map(); for (const [, block] of fn.body.blocks) { @@ -49,7 +55,8 @@ function getContextReassignment( let reassignment = getContextReassignment( value.loweredFunc.func, contextVariables, - true + true, + isAsync || value.loweredFunc.func.async ); if (reassignment === null) { // If the function itself doesn't reassign, does one of its dependencies? @@ -65,6 +72,18 @@ function getContextReassignment( } // if the function or its depends reassign, propagate that fact on the lvalue if (reassignment !== null) { + if (isAsync || value.loweredFunc.func.async) { + CompilerError.throwInvalidReact({ + reason: + "Reassigning a variable in an async function can cause inconsistent behavior on subsequent renders. Consider using state instead", + description: + reassignment.identifier.name !== null && + reassignment.identifier.name.kind === "named" + ? `Variable \`${reassignment.identifier.name.value}\` cannot be reassigned after render` + : "", + loc: reassignment.loc, + }); + } reassigningFunctions.set(lvalue.identifier.id, reassignment); } break; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-local-variable-in-async-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-local-variable-in-async-callback.expect.md new file mode 100644 index 0000000000000..15c631f420584 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-local-variable-in-async-callback.expect.md @@ -0,0 +1,37 @@ + +## Input + +```javascript +function Component() { + let value = null; + const reassign = async () => { + await foo().then((result) => { + // Reassigning a local variable in an async function is *always* mutating + // after render, so this should error regardless of where this ends up + // getting called + value = result; + }); + }; + + const onClick = async () => { + await reassign(); + }; + return
Click
; +} + +``` + + +## Error + +``` + 6 | // after render, so this should error regardless of where this ends up + 7 | // getting called +> 8 | value = result; + | ^^^^^ 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) + 9 | }); + 10 | }; + 11 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-async-callback.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-local-variable-in-async-callback.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-async-callback.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-reassign-local-variable-in-async-callback.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-async-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-async-callback.expect.md deleted file mode 100644 index aa07cce436136..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-async-callback.expect.md +++ /dev/null @@ -1,58 +0,0 @@ - -## Input - -```javascript -function Component() { - let value = null; - const reassign = async () => { - await foo().then((result) => { - // Reassigning a local variable in an async function is *always* mutating - // after render, so this should error regardless of where this ends up - // getting called - value = result; - }); - }; - - const onClick = async () => { - await reassign(); - }; - return
Click
; -} - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -function Component() { - const $ = _c(2); - let value; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - value = null; - $[0] = value; - } else { - value = $[0]; - } - let t0; - if ($[1] === Symbol.for("react.memo_cache_sentinel")) { - const reassign = async () => { - await foo().then((result) => { - value = result; - }); - }; - - const onClick = async () => { - await reassign(); - }; - - t0 =
Click
; - $[1] = t0; - } else { - t0 = $[1]; - } - return t0; -} - -``` - \ No newline at end of file diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 1bc208a53ed07..85f7413534bbe 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -483,12 +483,6 @@ const skipFilter = new Set([ "rules-of-hooks/rules-of-hooks-93dc5d5e538a", "rules-of-hooks/rules-of-hooks-69521d94fa03", - // should error - "todo.invalid-reassign-local-variable-in-jsx-callback", - "todo.invalid-reassign-local-variable-in-hook-argument", - "todo.invalid-reassign-local-variable-in-effect", - "todo.invalid-reassign-local-variable-in-async-callback", - // bugs "bug-invalid-hoisting-functionexpr", "original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block",