Don't flush synchronous work if we're in the middle of a ViewTransition async sequence #32760
Conversation
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
github-actions
bot
added
the
React Core Team
acdlite
approved these changes
Mar 26, 2025
|
Comparing: 5f232d7...0da7164 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
This unifies the pattern with startGestureTransition that already did this so that we can cancel it if needed.
This ensures that we don't play a partial animation when we're interrupted.
…on async sequence
sebmarkbage
force-pushed
the
abortedtransition
branch
from
0fb1a7b to
340c94f
Compare
We already had an equivalent helper that we already used in root scheduler.
|
@jherr I believe this will fix the issue you found. Doesn't have to do with the heuristics but rather the tradeoff between synchronous execution vs View Transitions. |
github-actions bot
pushed a commit
that referenced
this pull request
Mar 26, 2025
…on async sequence (#32760) Starting a View Transition is an async sequence. Since React can get a sync update in the middle of sequence we sometimes interrupt that sequence. Currently, we don't actually cancel the View Transition so it can just run as a partial. This ensures that we fully skip it when that happens, as well as warn. However, it's very easy to trigger this with just a setState in useLayoutEffect right now. Therefore if we're inside the preparing sequence of a startViewTransition, this delays work that would've normally flushed in a microtask. ~Maybe we want to do the same for Default work already scheduled through a scheduler Task.~ Edit: This was already done. `flushSync` currently will still lead to an interrupted View Transition (with a warning). There's a tradeoff here whether we want to try our best to preserve the guarantees of `flushSync` or favor the animation. It's already possible to suspend at the root with `flushSync` which means it's not always 100% guaranteed to commit anyway. We could treat it as suspended. But let's see how much this is a problem in practice. DiffTrain build for [a5297ec](a5297ec)
github-actions bot
pushed a commit
that referenced
this pull request
Mar 26, 2025
…on async sequence (#32760) Starting a View Transition is an async sequence. Since React can get a sync update in the middle of sequence we sometimes interrupt that sequence. Currently, we don't actually cancel the View Transition so it can just run as a partial. This ensures that we fully skip it when that happens, as well as warn. However, it's very easy to trigger this with just a setState in useLayoutEffect right now. Therefore if we're inside the preparing sequence of a startViewTransition, this delays work that would've normally flushed in a microtask. ~Maybe we want to do the same for Default work already scheduled through a scheduler Task.~ Edit: This was already done. `flushSync` currently will still lead to an interrupted View Transition (with a warning). There's a tradeoff here whether we want to try our best to preserve the guarantees of `flushSync` or favor the animation. It's already possible to suspend at the root with `flushSync` which means it's not always 100% guaranteed to commit anyway. We could treat it as suspended. But let's see how much this is a problem in practice. DiffTrain build for [a5297ec](a5297ec)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Starting a View Transition is an async sequence. Since React can get a sync update in the middle of sequence we sometimes interrupt that sequence.
Currently, we don't actually cancel the View Transition so it can just run as a partial. This ensures that we fully skip it when that happens, as well as warn.
However, it's very easy to trigger this with just a setState in useLayoutEffect right now. Therefore if we're inside the preparing sequence of a startViewTransition, this delays work that would've normally flushed in a microtask.
Maybe we want to do the same for Default work already scheduled through a scheduler Task.Edit: This was already done.flushSynccurrently will still lead to an interrupted View Transition (with a warning). There's a tradeoff here whether we want to try our best to preserve the guarantees offlushSyncor favor the animation. It's already possible to suspend at the root withflushSyncwhich means it's not always 100% guaranteed to commit anyway. We could treat it as suspended. But let's see how much this is a problem in practice.