[Fiber] render boundary in fallback if it contains a new stylesheet during sync update #28965
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
|
Comparing: 0a0a5c0...8839e50 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
reviewed
May 10, 2024
| workInProgress.flags |= ShouldSuspendCommit; | ||
| } else { | ||
| const rootRenderLanes = getWorkInProgressRootRenderLanes(); | ||
| if (!includesOnlyNonUrgentLanes(rootRenderLanes) && required) { |
There was a problem hiding this comment.
I don't really like introducing this concept of required and not-required resources. It also only gets read when you're rendering a new tree, not during an update; when you rendering a new tree, it's pretty much always fine to show a fallback, even if the render is synchronous.
The case where you might want to distinguish between "required" and "not required" resources is during a sync update, when the choice is between a stale resource or replacing the existing UI with a fallback (bad).
But that can be modeled during the waitForCommitToBeReady phase by effectively assigning this resource a timeout of 0. I think that's the better strategy.
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
da0b480
to
946168d
Compare
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
946168d
to
af03a72
Compare
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
af03a72
to
13b9396
Compare
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
13b9396
to
1798c2c
Compare
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
1798c2c
to
27bf255
Compare
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
27bf255
to
8596091
Compare
acdlite
approved these changes
May 21, 2024
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
8596091
to
68d05e4
Compare
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
68d05e4
to
3cf4ab1
Compare
…uring sync update When we implemented Suspensey CSS we had a heuristic that if the update was sync we would ignore the loading states of any new stylesheets and just do the commit. But for a stylesheet capability to be useful it needs to reliably prevent FOUC and since the stylesheet api is opt-in through precedence we don't have to maintain backaward compat (old stylesheets do not block commit but then nobody really renders them because of FOUC anyway) This update modifies the logic to put a boundary back into fallback if a sync update would lead to a stylesheet commiting before it loaded.
gnoff
force-pushed
the
suspend-boundary-on-sync-stylesheet
branch
from
3cf4ab1
to
8839e50
Compare
github-actions bot
pushed a commit
that referenced
this pull request
May 21, 2024
…uring sync update (#28965) Updates Suspensey instances and resources to preload even during urgent updates and to potentially suspend. The current implementation is unchanged for transitions but for sync updates if there is a suspense boundary above the resource/instance it will be rendered in fallback mode instead. Note: This behavior is not what we want for images once we make them suspense enabled. We will need to have forked behavior here to distinguish between stylesheets which should never commit when not loaded and images which should commit after a small delay DiffTrain build for commit 217b2cc.
github-actions bot
pushed a commit
that referenced
this pull request
May 21, 2024
…uring sync update (#28965) Updates Suspensey instances and resources to preload even during urgent updates and to potentially suspend. The current implementation is unchanged for transitions but for sync updates if there is a suspense boundary above the resource/instance it will be rendered in fallback mode instead. Note: This behavior is not what we want for images once we make them suspense enabled. We will need to have forked behavior here to distinguish between stylesheets which should never commit when not loaded and images which should commit after a small delay DiffTrain build for [217b2cc](217b2cc)
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.
Updates Suspensey instances and resources to preload even during urgent updates and to potentially suspend.
The current implementation is unchanged for transitions but for sync updates if there is a suspense boundary above the resource/instance it will be rendered in fallback mode instead.
Note: This behavior is not what we want for images once we make them suspense enabled. We will need to have forked behavior here to distinguish between stylesheets which should never commit when not loaded and images which should commit after a small delay