Skip to content

[Fizz] Handle nested SuspenseList #33308

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

Merged
merged 1 commit into from
May 20, 2025

Conversation

sebmarkbage
Copy link
Collaborator

Follow up to #33306.

If we're nested inside a SuspenseList and we have a row, then we can point our last row to block the parent row and unblock the parent when the last child unblocks.

@sebmarkbage sebmarkbage requested a review from eps1lon May 19, 2025 21:37
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label May 19, 2025
@react-sizebot
Copy link

react-sizebot commented May 19, 2025

Comparing: c6c2a52...54e2d36

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB +0.11% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 529.74 kB 529.74 kB = 93.49 kB 93.49 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 651.48 kB 651.48 kB = 114.75 kB 114.76 kB
facebook-www/ReactDOM-prod.classic.js = 675.72 kB 675.72 kB = 118.84 kB 118.85 kB
facebook-www/ReactDOM-prod.modern.js = 666.00 kB 666.00 kB = 117.23 kB 117.23 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 54e2d36

@sebmarkbage sebmarkbage force-pushed the fizznestedsuspenselist branch from 97adaed to 3314293 Compare May 19, 2025 22:54
@sebmarkbage sebmarkbage merged commit c4676e7 into facebook:main May 20, 2025
240 checks passed
sebmarkbage added a commit that referenced this pull request May 20, 2025
Stacked on #33308.

For "together" mode, we can be a self-blocking row that adds all its
boundaries to the blocked set, but there's no parent row that unblocks
it.

A particular quirk of this mode is that it's not enough to just unblock
them all on the server together. Because if one boundary downloads all
its html and then issues a complete instruction it'll appear before the
others while streaming in. What we actually want is to reveal them all
in a single batch.

This implementation takes a short cut by unblocking the rows in
`flushPartialBoundary`. That ensures that all the segments of every
boundary has a chance to flush before we start emitting any of the
complete boundary instructions. Once the last one unblocks, all the
complete boundary instructions are queued. Ideally this would be a
single `<script>` tag so that they can't be split up even if we get a
chunk containing some of them.

~A downside of this approach is that we always outline these boundaries.
We could inline them if they all complete before the parent flushes.
E.g. by checking if the row is blocked only by its own boundaries and if
all the boundaries would fit without getting outlined, then we can
inline them all at once.~ I went ahead and did this because it solves an
issue with `renderToString` where it doesn't support the script runtime
so it can only handle this if inlined.
github-actions bot pushed a commit that referenced this pull request May 20, 2025
Stacked on #33308.

For "together" mode, we can be a self-blocking row that adds all its
boundaries to the blocked set, but there's no parent row that unblocks
it.

A particular quirk of this mode is that it's not enough to just unblock
them all on the server together. Because if one boundary downloads all
its html and then issues a complete instruction it'll appear before the
others while streaming in. What we actually want is to reveal them all
in a single batch.

This implementation takes a short cut by unblocking the rows in
`flushPartialBoundary`. That ensures that all the segments of every
boundary has a chance to flush before we start emitting any of the
complete boundary instructions. Once the last one unblocks, all the
complete boundary instructions are queued. Ideally this would be a
single `<script>` tag so that they can't be split up even if we get a
chunk containing some of them.

~A downside of this approach is that we always outline these boundaries.
We could inline them if they all complete before the parent flushes.
E.g. by checking if the row is blocked only by its own boundaries and if
all the boundaries would fit without getting outlined, then we can
inline them all at once.~ I went ahead and did this because it solves an
issue with `renderToString` where it doesn't support the script runtime
so it can only handle this if inlined.

DiffTrain build for [99aa685](99aa685)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants