Bugfix: useDeferredValue loop during popstate transition#27559
Merged
acdlite merged 2 commits intofacebook:mainfrom Oct 21, 2023
Merged
Bugfix: useDeferredValue loop during popstate transition#27559acdlite merged 2 commits intofacebook:mainfrom
acdlite merged 2 commits intofacebook:mainfrom
Conversation
Adds a regression test where an unstable value passed to useDeferredValue value during a popstate transition can lead to an infinite render loop.
During a popstate event, we attempt to render updates synchronously even if they are transitions, to preserve scroll position if possible. We do this by entangling the transition lane with the Sync lane. However, if rendering the transition spawns additional transition updates (e.g. a setState inside useEffect), there's no reason to render those synchronously, too. We should use the normal transition behavior. This fixes an issue where useDeferredValue during a popstate event would spawn a transition update that was itself also synchronous.
facebook-github-bot
added
CLA Signed
React Core Team
|
Comparing: f172fa7...c9f5737 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
Collaborator
|
What if you update two roots in one popstate? |
Collaborator
Author
|
meh it just won’t preserve scroll position |
tyao1
approved these changes
Oct 21, 2023
Collaborator
Author
|
We could support that but I think to do it properly we'd need to track either |
github-actions bot
pushed a commit
that referenced
this pull request
Oct 21, 2023
During a popstate event, we attempt to render updates synchronously even if they are transitions, to preserve scroll position if possible. We do this by entangling the transition lane with the Sync lane. However, if rendering the transition spawns additional transition updates (e.g. a setState inside useEffect), there's no reason to render those synchronously, too. We should use the normal transition behavior. This fixes an issue where useDeferredValue during a popstate event would spawn a transition update that was itself also synchronous. DiffTrain build for [6db7f42](6db7f42)
acdlite
added a commit
to acdlite/next.js
that referenced
this pull request
Oct 23, 2023
acdlite
added a commit
to acdlite/next.js
that referenced
this pull request
Oct 23, 2023
kodiakhq bot
pushed a commit
to vercel/next.js
that referenced
this pull request
Oct 23, 2023
React upstream changes: - facebook/react#27570 - facebook/react#27569 - facebook/react#27550 - facebook/react#27559 - facebook/react#27552 - facebook/react#27504 - facebook/react#27522 Co-authored-by: Josh Story <2716369+gnoff@users.noreply.github.com>
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
) During a popstate event, we attempt to render updates synchronously even if they are transitions, to preserve scroll position if possible. We do this by entangling the transition lane with the Sync lane. However, if rendering the transition spawns additional transition updates (e.g. a setState inside useEffect), there's no reason to render those synchronously, too. We should use the normal transition behavior. This fixes an issue where useDeferredValue during a popstate event would spawn a transition update that was itself also synchronous.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
During a popstate event, we attempt to render updates synchronously even if they are transitions, to preserve scroll position if possible. We do this by entangling the transition lane with the Sync lane. However, if rendering the transition spawns additional transition updates (e.g. a setState inside useEffect), there's no reason to render those synchronously, too. We should use the normal transition behavior. This fixes an issue where useDeferredValue during a popstate event would spawn a transition update that was itself also synchronous. DiffTrain build for commit 6db7f42.
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
5 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.
During a popstate event, we attempt to render updates synchronously even if they are transitions, to preserve scroll position if possible. We do this by entangling the transition lane with the Sync lane.
However, if rendering the transition spawns additional transition updates (e.g. a setState inside useEffect), there's no reason to render those synchronously, too. We should use the normal transition behavior.
This fixes an issue where useDeferredValue during a popstate event would spawn a transition update that was itself also synchronous.