-
Notifications
You must be signed in to change notification settings - Fork 47k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fizz] Replay Postponed Paths #27379
Merged
Merged
+1,211
−219
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
A ReplayTask is a task that isn't writing to the output but follows a resumable path. Most of the code paths will be reused for this type of work.
If we have ResumeSlots we don't have an element key to match them with. This only happens inside an array. If we have any, we can simply pick those children out of the array and resume them by rendering that node normally into the relevant segment.
facebook-github-bot
added
CLA Signed
React Core Team
Opened by a member of the React Core Team
labels
Sep 14, 2023
It's like a combination of ReplaySuspenseBoundary and ResumeElement.
sebmarkbage
force-pushed
the
replaying
branch
from
September 14, 2023 23:16
9c5f863
to
5b276c6
Compare
acdlite
approved these changes
Sep 15, 2023
github-actions bot
pushed a commit
that referenced
this pull request
Sep 15, 2023
This forks Task into ReplayTask and RenderTask. A RenderTask is the normal mode and it has a segment to write into. A ReplayTask doesn't have a segment to write into because that has already been written but instead it has a ReplayState which keeps track of the next set of paths to follow. Once we hit a "Resume" node we convert it into a RenderTask and continue rendering from there. We can resume at either an Element position or a Slot position. An Element pointing to a component doesn't mean we resume that component, it means we resume in the child position directly below that component. Slots are slots inside arrays. Instead of statically forking most paths, I kept using the same path and checked for the existence of a segment or replay state dynamically at runtime. However, there's still quite a bit of forking here like retryRenderTask and retryReplayTask. Even in the new forks there's a lot of duplication like resumeSuspenseBoundary, replaySuspenseBoundary and renderSuspenseBoundary. There's opportunity to simplify this a bit. DiffTrain build for [a5fc797](a5fc797)
gnoff
approved these changes
Sep 15, 2023
Comment on lines
+1005
to
+1011
if (enableFloat) { | ||
// Does this even matter for replaying? | ||
setCurrentlyRenderingBoundaryResourcesTarget( | ||
request.renderState, | ||
resumedBoundary.resources, | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A child of this boundary may need to render and during that render new styles could be discovered so we'd want to associate those to this boundary. Makes sense to me that this is still needed
kodiakhq bot
pushed a commit
to vercel/next.js
that referenced
this pull request
Sep 20, 2023
Update React from d6dcad6a8 to 2807d781a. ### React upstream changes - facebook/react#27387 - facebook/react#27386 - facebook/react#27385 - facebook/react#27379 - facebook/react#27382 Closes NEXT-1637
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
This forks Task into ReplayTask and RenderTask. A RenderTask is the normal mode and it has a segment to write into. A ReplayTask doesn't have a segment to write into because that has already been written but instead it has a ReplayState which keeps track of the next set of paths to follow. Once we hit a "Resume" node we convert it into a RenderTask and continue rendering from there. We can resume at either an Element position or a Slot position. An Element pointing to a component doesn't mean we resume that component, it means we resume in the child position directly below that component. Slots are slots inside arrays. Instead of statically forking most paths, I kept using the same path and checked for the existence of a segment or replay state dynamically at runtime. However, there's still quite a bit of forking here like retryRenderTask and retryReplayTask. Even in the new forks there's a lot of duplication like resumeSuspenseBoundary, replaySuspenseBoundary and renderSuspenseBoundary. There's opportunity to simplify this a bit.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
This forks Task into ReplayTask and RenderTask. A RenderTask is the normal mode and it has a segment to write into. A ReplayTask doesn't have a segment to write into because that has already been written but instead it has a ReplayState which keeps track of the next set of paths to follow. Once we hit a "Resume" node we convert it into a RenderTask and continue rendering from there. We can resume at either an Element position or a Slot position. An Element pointing to a component doesn't mean we resume that component, it means we resume in the child position directly below that component. Slots are slots inside arrays. Instead of statically forking most paths, I kept using the same path and checked for the existence of a segment or replay state dynamically at runtime. However, there's still quite a bit of forking here like retryRenderTask and retryReplayTask. Even in the new forks there's a lot of duplication like resumeSuspenseBoundary, replaySuspenseBoundary and renderSuspenseBoundary. There's opportunity to simplify this a bit. DiffTrain build for commit a5fc797.
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.
This forks Task into ReplayTask and RenderTask.
A RenderTask is the normal mode and it has a segment to write into.
A ReplayTask doesn't have a segment to write into because that has already been written but instead it has a ReplayState which keeps track of the next set of paths to follow. Once we hit a "Resume" node we convert it into a RenderTask and continue rendering from there.
We can resume at either an Element position or a Slot position. An Element pointing to a component doesn't mean we resume that component, it means we resume in the child position directly below that component. Slots are slots inside arrays.
Instead of statically forking most paths, I kept using the same path and checked for the existence of a segment or replay state dynamically at runtime.
However, there's still quite a bit of forking here like retryRenderTask and retryReplayTask. Even in the new forks there's a lot of duplication like resumeSuspenseBoundary, replaySuspenseBoundary and renderSuspenseBoundary. There's opportunity to simplify this a bit.