[compiler] Allow setState after await in effect functions#36734
Open
poteto wants to merge 2 commits into
Open
Conversation
ValidateNoSetStateInEffects flags setState calls made after an await in an async function invoked from a useEffect, even though post-await code runs in a microtask rather than synchronously in the effect body, so the cascading render rationale does not apply. Documents the current wrong behavior for react#34905: the lint-mode logger output shows the EffectSetState error pointing at load() despite the setState only ever running after the await.
ValidateNoSetStateInEffects treated every setState call inside a function transitively invoked from a useEffect body as a synchronous cascading render, including calls that can only execute after an await. Code after an await resumes in a microtask after the effect body has returned, so the cascading-render rationale does not apply. Fixes react#34905. Both compilers now compute a forward must-dataflow over the HIR CFG and suppress a setState iff every path from the function entry to the call passes through an Await. A naive block-local seenAwait flag would be unsound: an await in one branch of a conditional must not suppress a setState that is also reachable through the other, await-free branch, so the analysis intersects over all predecessors. The new invalid-setState-in-useEffect-after-conditional-await fixture documents that a conditional await still flags, and invalid-setState-in-useEffect-before-await covers setState ahead of the first await. Exception edges are safe to treat like normal edges because HIRBuilder.push gives every instruction inside a try region its own maybe-throw block, and an Await's rejection also resumes in a microtask. Adopts the post-await fixpoint approach from react#36417, reworked to track per-block state in both the TypeScript and Rust implementations. Co-authored-by: Raashish Aggarwal <94279692+raashish1601@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
react-hooks/set-state-in-effectflagged setState calls that happen after anawaitinside an async function invoked from an effect. Post-await code resumes in a microtask after the effect body has returned, so the "synchronous setState cascades a render" rationale does not apply; the lint was a false positive on a common data-loading shape (15 reactions on the issue).The fix exempts a setState only when it is provably post-await: a forward must-dataflow over the HIR CFG computes the blocks that begin after an await has executed on every path from the function entry (optimistic initialization so loop back-edges do not pessimize the meet, fixpoint to the greatest solution), plus an intra-block flag for instructions after an Await in the same block. A setState reachable on any await-free path still flags, so the conditional-await case remains an error by design, with a fixture documenting that choice. Suppression is sound under try/catch because HIRBuilder terminates blocks after each instruction in a try region, and an awaited rejection also resumes in a microtask.
Fixtures: post-await setState (event gone), setState before the first await (still flags), setState after a conditional await (still flags). First commit documents the false positive via the lint-mode logger output, second removes it.
Builds on the approach in #36417 by @raashish1601, hardened from a seen-await flag to the path-sensitive analysis above. Implemented identically in the TypeScript compiler and the Rust port.
Verification: TS snap 1807/1807, Rust snap 1807/1807, cargo workspace green, scoped TS-vs-Rust HIR parity harness green.
Closes #34905