Don't try to hydrate a hidden Offscreen tree #32862
Merged
+49
−1
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
|
Comparing: 39cad7a...c88cb05 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) |
sebmarkbage
force-pushed
the
offscreenhydrationbug
branch
from
13272eb to
c88cb05
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Apr 15, 2025
I found a bug even before the Activity hydration stuff. If we're hydrating an Offscreen boundary in its "hidden" state it won't have any content to hydrate so will trigger hydration errors (which are then eaten by the Offscreen boundary itself). Leaving it not prewarmed. This doesn't happen in the simple case because we'd be hydrating at a higher priority than Offscreen at the root, and those are deferred to Offscreen by not having higher priority. However, we've hydrating at the Offscreen priority, which we do inside Suspense boundaries, then it tries to hydrate against an empty set. I ended up moving this to the Activity boundary in a future PR since it's the SSR side that decided where to not render something and it only has a concept of Activity, no Offscreen. 1dc05a5#diff-d5166797ebbc5b646a49e6a06a049330ca617985d7a6edf3ad1641b43fde1ddfR1111 DiffTrain build for [b04254f](b04254f)
github-actions bot
pushed a commit
that referenced
this pull request
Apr 15, 2025
I found a bug even before the Activity hydration stuff. If we're hydrating an Offscreen boundary in its "hidden" state it won't have any content to hydrate so will trigger hydration errors (which are then eaten by the Offscreen boundary itself). Leaving it not prewarmed. This doesn't happen in the simple case because we'd be hydrating at a higher priority than Offscreen at the root, and those are deferred to Offscreen by not having higher priority. However, we've hydrating at the Offscreen priority, which we do inside Suspense boundaries, then it tries to hydrate against an empty set. I ended up moving this to the Activity boundary in a future PR since it's the SSR side that decided where to not render something and it only has a concept of Activity, no Offscreen. 1dc05a5#diff-d5166797ebbc5b646a49e6a06a049330ca617985d7a6edf3ad1641b43fde1ddfR1111 DiffTrain build for [b04254f](b04254f)
sebmarkbage
added a commit
that referenced
this pull request
Apr 23, 2025
Stacked on #32862 and #32842. This means that Activity boundaries now act as boundaries which can have their effects mounted independently. Just like Suspense boundaries, we hydrate the outer content first and then start hydrating the content in an Offscreen lane. Flowing props or interacting with the content increases the priority just like Suspense boundaries. This skips emitting even the comments for `<Activity mode="hidden">` so we don't hydrate those. Instead those are deferred to a later client render. The implementation are just forked copies of the SuspenseComponent branches and then carefully going through each line and tweaking it. The main interesting bit is that, unlike Suspense, Activity boundaries don't have fallbacks so all those branches where you might commit a suspended tree disappears. Instead, if something suspends while hydration, we can just leave the dehydrated content in place. However, if something does suspend during client rendering then it should bubble up to the parent. Therefore, we have to be careful to only pushSuspenseHandler when hydrating. That's really the main difference. This just uses the existing basic Activity tests but I've started work on port all of the applicable Suspense tests in SelectiveHydration-test and PartialHydration-test to Activity versions.
github-actions bot
pushed a commit
that referenced
this pull request
Apr 23, 2025
Stacked on #32862 and #32842. This means that Activity boundaries now act as boundaries which can have their effects mounted independently. Just like Suspense boundaries, we hydrate the outer content first and then start hydrating the content in an Offscreen lane. Flowing props or interacting with the content increases the priority just like Suspense boundaries. This skips emitting even the comments for `<Activity mode="hidden">` so we don't hydrate those. Instead those are deferred to a later client render. The implementation are just forked copies of the SuspenseComponent branches and then carefully going through each line and tweaking it. The main interesting bit is that, unlike Suspense, Activity boundaries don't have fallbacks so all those branches where you might commit a suspended tree disappears. Instead, if something suspends while hydration, we can just leave the dehydrated content in place. However, if something does suspend during client rendering then it should bubble up to the parent. Therefore, we have to be careful to only pushSuspenseHandler when hydrating. That's really the main difference. This just uses the existing basic Activity tests but I've started work on port all of the applicable Suspense tests in SelectiveHydration-test and PartialHydration-test to Activity versions. DiffTrain build for [3ef31d1](3ef31d1)
github-actions bot
pushed a commit
that referenced
this pull request
Apr 23, 2025
Stacked on #32862 and #32842. This means that Activity boundaries now act as boundaries which can have their effects mounted independently. Just like Suspense boundaries, we hydrate the outer content first and then start hydrating the content in an Offscreen lane. Flowing props or interacting with the content increases the priority just like Suspense boundaries. This skips emitting even the comments for `<Activity mode="hidden">` so we don't hydrate those. Instead those are deferred to a later client render. The implementation are just forked copies of the SuspenseComponent branches and then carefully going through each line and tweaking it. The main interesting bit is that, unlike Suspense, Activity boundaries don't have fallbacks so all those branches where you might commit a suspended tree disappears. Instead, if something suspends while hydration, we can just leave the dehydrated content in place. However, if something does suspend during client rendering then it should bubble up to the parent. Therefore, we have to be careful to only pushSuspenseHandler when hydrating. That's really the main difference. This just uses the existing basic Activity tests but I've started work on port all of the applicable Suspense tests in SelectiveHydration-test and PartialHydration-test to Activity versions. DiffTrain build for [3ef31d1](3ef31d1)
github-actions bot
pushed a commit
to code/lib-react
that referenced
this pull request
Apr 23, 2025
Stacked on facebook#32862 and facebook#32842. This means that Activity boundaries now act as boundaries which can have their effects mounted independently. Just like Suspense boundaries, we hydrate the outer content first and then start hydrating the content in an Offscreen lane. Flowing props or interacting with the content increases the priority just like Suspense boundaries. This skips emitting even the comments for `<Activity mode="hidden">` so we don't hydrate those. Instead those are deferred to a later client render. The implementation are just forked copies of the SuspenseComponent branches and then carefully going through each line and tweaking it. The main interesting bit is that, unlike Suspense, Activity boundaries don't have fallbacks so all those branches where you might commit a suspended tree disappears. Instead, if something suspends while hydration, we can just leave the dehydrated content in place. However, if something does suspend during client rendering then it should bubble up to the parent. Therefore, we have to be careful to only pushSuspenseHandler when hydrating. That's really the main difference. This just uses the existing basic Activity tests but I've started work on port all of the applicable Suspense tests in SelectiveHydration-test and PartialHydration-test to Activity versions. DiffTrain build for [3ef31d1](facebook@3ef31d1)
github-actions bot
pushed a commit
to code/lib-react
that referenced
this pull request
Apr 23, 2025
Stacked on facebook#32862 and facebook#32842. This means that Activity boundaries now act as boundaries which can have their effects mounted independently. Just like Suspense boundaries, we hydrate the outer content first and then start hydrating the content in an Offscreen lane. Flowing props or interacting with the content increases the priority just like Suspense boundaries. This skips emitting even the comments for `<Activity mode="hidden">` so we don't hydrate those. Instead those are deferred to a later client render. The implementation are just forked copies of the SuspenseComponent branches and then carefully going through each line and tweaking it. The main interesting bit is that, unlike Suspense, Activity boundaries don't have fallbacks so all those branches where you might commit a suspended tree disappears. Instead, if something suspends while hydration, we can just leave the dehydrated content in place. However, if something does suspend during client rendering then it should bubble up to the parent. Therefore, we have to be careful to only pushSuspenseHandler when hydrating. That's really the main difference. This just uses the existing basic Activity tests but I've started work on port all of the applicable Suspense tests in SelectiveHydration-test and PartialHydration-test to Activity versions. DiffTrain build for [3ef31d1](facebook@3ef31d1)
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.
I found a bug even before the Activity hydration stuff.
If we're hydrating an Offscreen boundary in its "hidden" state it won't have any content to hydrate so will trigger hydration errors (which are then eaten by the Offscreen boundary itself). Leaving it not prewarmed.
This doesn't happen in the simple case because we'd be hydrating at a higher priority than Offscreen at the root, and those are deferred to Offscreen by not having higher priority. However, we've hydrating at the Offscreen priority, which we do inside Suspense boundaries, then it tries to hydrate against an empty set.
I ended up moving this to the Activity boundary in a future PR since it's the SSR side that decided where to not render something and it only has a concept of Activity, no Offscreen.
1dc05a5#diff-d5166797ebbc5b646a49e6a06a049330ca617985d7a6edf3ad1641b43fde1ddfR1111