useDeferredValue should skip initialValue if it suspends #27509
Merged
Conversation
This file contains 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
facebook-github-bot
added
CLA Signed
React Core Team
acdlite
force-pushed
the
udv-spawn-on-suspend
branch
from
1e1d2eb
to
63a171b
Compare
|
Comparing: 9abf6fa...66267d0 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 |
acdlite
force-pushed
the
udv-spawn-on-suspend
branch
2 times, most recently
from
8999797
to
89e8b89
Compare
acdlite
commented
Oct 13, 2023
| if (includesOnlyRetries(workInProgressRootRenderLanes)) { | ||
| // Retries are slightly lower priority than transitions, so if Retry task | ||
| // spawns a deferred task, the deferred task is also considered a Retry. | ||
| workInProgressDeferredLane = claimNextRetryLane(); |
There was a problem hiding this comment.
Still thinking about this... I'm not sure it's quite right because elsewhere we assume that a Retry task is only used to fill in Suspense fallbacks.
The only reason I have a special case here is because Retry is lower priority than Transitions, and I wanted to avoid a priority inversion. So it's mostly theoretical.
There was a problem hiding this comment.
yeah I'm going to revert this part and spawn as a Transition (like it does today) until I can think of a more compelling reason why it should be different.
I'll leave the Offscreen branch, though.
acdlite
force-pushed
the
udv-spawn-on-suspend
branch
from
89e8b89
to
67770c0
Compare
sebmarkbage
approved these changes
Oct 16, 2023
The Deferred lane will be used to mark deferred work that is spawned by useDeferredValue during render. It doesn't have a priority on its own; it will always be entangled with some other lane. That's also why we don't need multiple deferred lanes. We can check for the presence of the Deferred lane to quickly determine if a render was spawned by useDeferredValue.
acdlite
force-pushed
the
udv-spawn-on-suspend
branch
from
67770c0
to
508437f
Compare
If a parent render spawns a deferred task with useDeferredValue, but the parent render suspends, we should not wait for the parent render to complete before attempting to render the final value. The reason is that the initialValue argument to useDeferredValue is meant to represent an immediate preview of the final UI. If we can't render it "immediately", we might as well skip it and go straight to the "real" value. This is an improvement over how a userspace implementation of useDeferredValue would work, because a userspace implementation would have to wait for the parent task to commit (useEffect) before spawning the deferred task, creating a waterfall.
acdlite
force-pushed
the
udv-spawn-on-suspend
branch
from
508437f
to
66267d0
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Oct 17, 2023
### Based on #27505 If a parent render spawns a deferred task with useDeferredValue, but the parent render suspends, we should not wait for the parent render to complete before attempting to render the final value. The reason is that the initialValue argument to useDeferredValue is meant to represent an immediate preview of the final UI. If we can't render it "immediately", we might as well skip it and go straight to the "real" value. This is an improvement over how a userspace implementation of useDeferredValue would work, because a userspace implementation would have to wait for the parent task to commit (useEffect) before spawning the deferred task, creating a waterfall. DiffTrain build for [b2a68a6](b2a68a6)
acdlite
added a commit
that referenced
this pull request
Oct 17, 2023
### Based on #27509 Revealing a prerendered tree (hidden -> visible) is considered the same as mounting a brand new tree. So, when an initialValue argument is passed to useDeferredValue, and it's prerendered inside a hidden tree, we should first prerender the initial value. After the initial value has been prerendered, we switch to prerendering the final one. This is the same sequence that we use when mounting new visible tree. Depending on how much prerendering work has been finished by the time the tree is revealed, we may or may not be able to skip all the way to the final value. This means we get the benefits of both prerendering and preview states: if we have enough resources to prerender the whole thing, we do that. If we don't, we have a preview state to show for immediate feedback.
github-actions bot
pushed a commit
that referenced
this pull request
Oct 17, 2023
### Based on #27509 Revealing a prerendered tree (hidden -> visible) is considered the same as mounting a brand new tree. So, when an initialValue argument is passed to useDeferredValue, and it's prerendered inside a hidden tree, we should first prerender the initial value. After the initial value has been prerendered, we switch to prerendering the final one. This is the same sequence that we use when mounting new visible tree. Depending on how much prerendering work has been finished by the time the tree is revealed, we may or may not be able to skip all the way to the final value. This means we get the benefits of both prerendering and preview states: if we have enough resources to prerender the whole thing, we do that. If we don't, we have a preview state to show for immediate feedback. DiffTrain build for [75c1bd7](75c1bd7)
kodiakhq bot
pushed a commit
to vercel/next.js
that referenced
this pull request
Oct 18, 2023
Update React from from 09fbee89d to a41957507. ### React upstream changes - facebook/react#27472 - facebook/react#27512 - facebook/react#27509 - facebook/react#27517 - facebook/react#27523 - facebook/react#27516 - facebook/react#27505
hoxyq
added a commit
that referenced
this pull request
Oct 18, 2023
Changes: * fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook ([hoxyq](https://github.com/hoxyq) in [#27472](#27472)) * useDeferredValue should skip initialValue if it suspends ([acdlite](https://github.com/acdlite) in [#27509](#27509)) * feat[react-devtools-extensions/logging]: initialize session id on the client for logging ([hoxyq](https://github.com/hoxyq) in [#27517](#27517)) * refactor[react-devtools-extensions]: use globals to eliminate dead code ([hoxyq](https://github.com/hoxyq) in [#27516](#27516)) * fix[devtools/inspectElement]: dont pause initial inspectElement call when user switches tabs ([hoxyq](https://github.com/hoxyq) in [#27488](#27488))
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
) ### Based on facebook#27505 If a parent render spawns a deferred task with useDeferredValue, but the parent render suspends, we should not wait for the parent render to complete before attempting to render the final value. The reason is that the initialValue argument to useDeferredValue is meant to represent an immediate preview of the final UI. If we can't render it "immediately", we might as well skip it and go straight to the "real" value. This is an improvement over how a userspace implementation of useDeferredValue would work, because a userspace implementation would have to wait for the parent task to commit (useEffect) before spawning the deferred task, creating a waterfall.
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
### Based on facebook#27509 Revealing a prerendered tree (hidden -> visible) is considered the same as mounting a brand new tree. So, when an initialValue argument is passed to useDeferredValue, and it's prerendered inside a hidden tree, we should first prerender the initial value. After the initial value has been prerendered, we switch to prerendering the final one. This is the same sequence that we use when mounting new visible tree. Depending on how much prerendering work has been finished by the time the tree is revealed, we may or may not be able to skip all the way to the final value. This means we get the benefits of both prerendering and preview states: if we have enough resources to prerender the whole thing, we do that. If we don't, we have a preview state to show for immediate feedback.
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
Changes: * fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook ([hoxyq](https://github.com/hoxyq) in [facebook#27472](facebook#27472)) * useDeferredValue should skip initialValue if it suspends ([acdlite](https://github.com/acdlite) in [facebook#27509](facebook#27509)) * feat[react-devtools-extensions/logging]: initialize session id on the client for logging ([hoxyq](https://github.com/hoxyq) in [facebook#27517](facebook#27517)) * refactor[react-devtools-extensions]: use globals to eliminate dead code ([hoxyq](https://github.com/hoxyq) in [facebook#27516](facebook#27516)) * fix[devtools/inspectElement]: dont pause initial inspectElement call when user switches tabs ([hoxyq](https://github.com/hoxyq) in [facebook#27488](facebook#27488))
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
### Based on #27505 If a parent render spawns a deferred task with useDeferredValue, but the parent render suspends, we should not wait for the parent render to complete before attempting to render the final value. The reason is that the initialValue argument to useDeferredValue is meant to represent an immediate preview of the final UI. If we can't render it "immediately", we might as well skip it and go straight to the "real" value. This is an improvement over how a userspace implementation of useDeferredValue would work, because a userspace implementation would have to wait for the parent task to commit (useEffect) before spawning the deferred task, creating a waterfall. DiffTrain build for commit b2a68a6.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
### Based on #27509 Revealing a prerendered tree (hidden -> visible) is considered the same as mounting a brand new tree. So, when an initialValue argument is passed to useDeferredValue, and it's prerendered inside a hidden tree, we should first prerender the initial value. After the initial value has been prerendered, we switch to prerendering the final one. This is the same sequence that we use when mounting new visible tree. Depending on how much prerendering work has been finished by the time the tree is revealed, we may or may not be able to skip all the way to the final value. This means we get the benefits of both prerendering and preview states: if we have enough resources to prerender the whole thing, we do that. If we don't, we have a preview state to show for immediate feedback. DiffTrain build for commit 75c1bd7.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Based on #27505
If a parent render spawns a deferred task with useDeferredValue, but the parent render suspends, we should not wait for the parent render to complete before attempting to render the final value.
The reason is that the initialValue argument to useDeferredValue is meant to represent an immediate preview of the final UI. If we can't render it "immediately", we might as well skip it and go straight to the "real" value.
This is an improvement over how a userspace implementation of useDeferredValue would work, because a userspace implementation would have to wait for the parent task to commit (useEffect) before spawning the deferred task, creating a waterfall.