Skip to content
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] Client render the nearest child or parent suspense boundary if replay errors or is aborted #27386

Merged
merged 7 commits into from
Sep 18, 2023

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Sep 18, 2023

Based on #27385.

When we error or abort during replay, that doesn't actually error the component that errored because that has already rendered. The error only affects any child that is not yet completed. Therefore the error kind of gets thrown at the resumable point.

The resumable point might be a hole in the replay path, in which case throwing there errors the parent boundary just the same as if the replay component errored. If the hole is inside a deeper Suspense boundary though, then it's that Suspense boundary that gets client rendered. I.e. the child boundary. We can still finish any siblings.

In the shell all resumable points are inside a boundary since we must have finished the shell. Therefore if you error in the root, we just simply just turn all incomplete boundaries into client renders.

The idea for this check is that we shouldn't flush anything before we flush
the shell. That may or may not hold true in future formats like RN.

It is a problem for resuming because with resuming it's possible to have
root tasks that are used from resuming but the shell was already flushed
so we can have completed boundaries before the shell has fully resumed.

It's not technically necessary to bail early because there won't be anything
in partialBoundaries or completedBoundaries because nothing gets added
there unless the parent has already flushed.

It's not exactly slow to have to check the length of three arrays so it's
probably not a big deal.

Flush partials in an early preamble needs further consideration regardless.
…boundaries

Typically aborting or erroring a replaying boundary will just client
render it like normal. However, when it happens at the shell level, we
might not have discovered all the boundaries since we didn't replay down
to them. So we need to go find any remaining boundaries and trigger
a client render instruction for them.

We also shouldn't trigger a fatalError in these scenarios since it's not
fatal. We've already emitted the shell.
We shouldn't log an extra error for the shell itself. Only the errored
boundaries. Since we only replay paths down to resumable boundaries
there should be at least one that gets the log.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Sep 18, 2023
);
// If we're replaying through this pass, it means we're replaying through
// an already completed Suspense boundary. It's too late to do anything about it
// so we can just render through it.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to this PR but surfaced by the test cases.

Completed Suspense boundaries can be parents that we replay through but since they're already complete they don't add any features. It's just the same as if it's in the shell. All of it has already flushed.

@react-sizebot
Copy link

react-sizebot commented Sep 18, 2023

Comparing: a5fc797...65c7519

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.min.js = 166.62 kB 166.62 kB = 52.13 kB 52.13 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 176.05 kB 176.05 kB = 54.97 kB 54.97 kB
facebook-www/ReactDOM-prod.classic.js = 571.73 kB 571.73 kB = 100.64 kB 100.64 kB
facebook-www/ReactDOM-prod.modern.js = 555.46 kB 555.46 kB = 97.75 kB 97.75 kB
oss-stable-semver/react-server/cjs/react-server.development.js +3.50% 178.06 kB 184.30 kB +2.28% 41.05 kB 41.99 kB
oss-stable/react-server/cjs/react-server.development.js +3.50% 178.06 kB 184.30 kB +2.28% 41.05 kB 41.99 kB
oss-experimental/react-server/cjs/react-server.development.js +3.35% 194.39 kB 200.91 kB +2.17% 44.84 kB 45.82 kB
oss-stable-semver/react-server/cjs/react-server.production.min.js +3.35% 32.95 kB 34.06 kB +3.33% 10.32 kB 10.67 kB
oss-stable/react-server/cjs/react-server.production.min.js +3.35% 32.95 kB 34.06 kB +3.33% 10.32 kB 10.67 kB
oss-experimental/react-server/cjs/react-server.production.min.js +3.29% 35.95 kB 37.14 kB +3.19% 11.20 kB 11.56 kB
facebook-www/ReactDOMServer-prod.modern.js +2.47% 174.04 kB 178.33 kB +2.02% 30.70 kB 31.32 kB
facebook-www/ReactDOMServer-prod.classic.js +2.45% 174.98 kB 179.27 kB +2.01% 30.94 kB 31.57 kB
facebook-www/ReactDOMServerStreaming-prod.modern.js +2.28% 178.22 kB 182.27 kB +1.84% 31.99 kB 32.58 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/react-server/cjs/react-server.development.js +3.50% 178.06 kB 184.30 kB +2.28% 41.05 kB 41.99 kB
oss-stable/react-server/cjs/react-server.development.js +3.50% 178.06 kB 184.30 kB +2.28% 41.05 kB 41.99 kB
oss-experimental/react-server/cjs/react-server.development.js +3.35% 194.39 kB 200.91 kB +2.17% 44.84 kB 45.82 kB
oss-stable-semver/react-server/cjs/react-server.production.min.js +3.35% 32.95 kB 34.06 kB +3.33% 10.32 kB 10.67 kB
oss-stable/react-server/cjs/react-server.production.min.js +3.35% 32.95 kB 34.06 kB +3.33% 10.32 kB 10.67 kB
oss-experimental/react-server/cjs/react-server.production.min.js +3.29% 35.95 kB 37.14 kB +3.19% 11.20 kB 11.56 kB
facebook-www/ReactDOMServer-prod.modern.js +2.47% 174.04 kB 178.33 kB +2.02% 30.70 kB 31.32 kB
facebook-www/ReactDOMServer-prod.classic.js +2.45% 174.98 kB 179.27 kB +2.01% 30.94 kB 31.57 kB
facebook-www/ReactDOMServerStreaming-prod.modern.js +2.28% 178.22 kB 182.27 kB +1.84% 31.99 kB 32.58 kB
facebook-www/ReactDOMServerStreaming-dev.modern.js +1.74% 375.88 kB 382.41 kB +1.31% 81.44 kB 82.51 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.development.js +1.71% 363.80 kB 370.04 kB +1.20% 79.93 kB 80.88 kB
oss-stable/react-dom/cjs/react-dom-server.bun.development.js +1.71% 363.82 kB 370.06 kB +1.20% 79.95 kB 80.91 kB
facebook-www/ReactDOMServer-dev.modern.js +1.71% 380.69 kB 387.21 kB +1.31% 82.58 kB 83.66 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +1.71% 71.38 kB 72.60 kB +1.71% 21.39 kB 21.76 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.production.min.js +1.71% 71.54 kB 72.76 kB +1.55% 21.78 kB 22.11 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.development.js +1.70% 366.30 kB 372.54 kB +1.19% 80.35 kB 81.31 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.development.js +1.70% 366.33 kB 372.57 kB +1.20% 80.38 kB 81.34 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.development.js +1.70% 366.57 kB 372.81 kB +1.19% 80.83 kB 81.79 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js +1.70% 366.60 kB 372.84 kB +1.19% 80.86 kB 81.82 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.development.js +1.70% 366.98 kB 373.22 kB +1.19% 80.95 kB 81.91 kB
oss-stable/react-dom/cjs/react-dom-server.edge.development.js +1.70% 367.01 kB 373.25 kB +1.19% 80.97 kB 81.94 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.development.js +1.70% 383.96 kB 390.49 kB +1.20% 81.21 kB 82.18 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.development.js +1.70% 383.99 kB 390.51 kB +1.20% 81.23 kB 82.21 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.development.js +1.70% 384.23 kB 390.76 kB +1.18% 81.70 kB 82.67 kB
oss-stable/react-dom/umd/react-dom-server.browser.development.js +1.70% 384.26 kB 390.79 kB +1.18% 81.73 kB 82.69 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.development.js +1.70% 368.05 kB 374.29 kB +1.18% 80.86 kB 81.81 kB
oss-stable/react-dom/cjs/react-dom-server.node.development.js +1.70% 368.08 kB 374.32 kB +1.18% 80.89 kB 81.84 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.development.js +1.69% 368.21 kB 374.45 kB +1.19% 80.81 kB 81.77 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.development.js +1.69% 368.24 kB 374.48 kB +1.19% 80.83 kB 81.80 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js +1.69% 385.86 kB 392.38 kB +1.17% 84.60 kB 85.59 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +1.69% 67.08 kB 68.21 kB +1.76% 19.94 kB 20.29 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +1.69% 67.11 kB 68.24 kB +1.76% 19.97 kB 20.32 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.production.min.js +1.68% 67.24 kB 68.37 kB +1.67% 20.29 kB 20.63 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.production.min.js +1.68% 67.27 kB 68.40 kB +1.68% 20.31 kB 20.66 kB
facebook-www/ReactDOMServer-dev.classic.js +1.68% 388.12 kB 394.64 kB +1.28% 84.23 kB 85.31 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js +1.68% 388.37 kB 394.89 kB +1.16% 85.03 kB 86.02 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.production.min.js +1.68% 68.10 kB 69.24 kB +1.71% 20.88 kB 21.24 kB
oss-stable/react-dom/cjs/react-dom-server.browser.production.min.js +1.68% 68.12 kB 69.26 kB +1.72% 20.91 kB 21.27 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.development.js +1.68% 406.89 kB 413.71 kB +1.26% 85.96 kB 87.04 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.production.min.js +1.67% 68.24 kB 69.38 kB +1.69% 21.18 kB 21.54 kB
oss-stable/react-dom/umd/react-dom-server.browser.production.min.js +1.67% 68.26 kB 69.40 kB +1.68% 21.20 kB 21.56 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js +1.67% 390.27 kB 396.80 kB +1.16% 85.49 kB 86.48 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js +1.65% 394.22 kB 400.74 kB +1.13% 85.80 kB 86.77 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.development.js +1.65% 394.63 kB 401.15 kB +1.14% 85.92 kB 86.89 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.min.js +1.65% 74.29 kB 75.52 kB +1.58% 22.49 kB 22.85 kB
oss-experimental/react-dom/umd/react-dom-server.browser.development.js +1.65% 413.04 kB 419.86 kB +1.25% 86.70 kB 87.78 kB
oss-experimental/react-dom/umd/react-dom-server.browser.production.min.js +1.65% 74.42 kB 75.64 kB +1.52% 22.81 kB 23.16 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js +1.65% 395.52 kB 402.04 kB +1.13% 86.11 kB 87.09 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.min.js +1.54% 76.06 kB 77.23 kB +1.56% 22.95 kB 23.31 kB
oss-stable-semver/react-dom/cjs/react-dom-server.bun.production.min.js +1.52% 71.51 kB 72.59 kB +1.64% 21.47 kB 21.82 kB
oss-stable/react-dom/cjs/react-dom-server.bun.production.min.js +1.52% 71.53 kB 72.62 kB +1.64% 21.49 kB 21.84 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.min.js +1.51% 77.61 kB 78.78 kB +1.51% 23.26 kB 23.61 kB
oss-stable-semver/react-dom/cjs/react-dom-server.edge.production.min.js +1.49% 73.38 kB 74.47 kB +1.63% 22.44 kB 22.81 kB
oss-stable/react-dom/cjs/react-dom-server.edge.production.min.js +1.49% 73.41 kB 74.50 kB +1.63% 22.47 kB 22.83 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.production.min.js +1.49% 73.44 kB 74.53 kB +1.67% 22.49 kB 22.87 kB
oss-stable/react-dom/cjs/react-dom-server.node.production.min.js +1.49% 73.46 kB 74.56 kB +1.67% 22.52 kB 22.90 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.production.min.js +1.48% 73.06 kB 74.14 kB +1.64% 21.74 kB 22.09 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.production.min.js +1.48% 73.08 kB 74.17 kB +1.65% 21.76 kB 22.12 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.production.min.js +1.48% 79.80 kB 80.98 kB +1.54% 24.14 kB 24.51 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.min.js +1.47% 80.07 kB 81.25 kB +1.58% 24.30 kB 24.68 kB

Generated by 🚫 dangerJS against 65c7519

That's because if the child of the replay is a Suspense boundary we can
client there instead instead of the parent boundary. For the shell,
that will always be the case.
@sebmarkbage sebmarkbage merged commit 925c66a into facebook:main Sep 18, 2023
2 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 18, 2023
… replay errors or is aborted (#27386)

Based on #27385.

When we error or abort during replay, that doesn't actually error the
component that errored because that has already rendered. The error only
affects any child that is not yet completed. Therefore the error kind of
gets thrown at the resumable point.

The resumable point might be a hole in the replay path, in which case
throwing there errors the parent boundary just the same as if the replay
component errored. If the hole is inside a deeper Suspense boundary
though, then it's that Suspense boundary that gets client rendered. I.e.
the child boundary. We can still finish any siblings.

In the shell all resumable points are inside a boundary since we must
have finished the shell. Therefore if you error in the root, we just
simply just turn all incomplete boundaries into client renders.

DiffTrain build for [925c66a](925c66a)
Comment on lines +3135 to +3137
if (resumedBoundary.parentFlushed) {
request.clientRenderedBoundaries.push(resumedBoundary);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we know the parent flushed in this case already

kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request Sep 20, 2023
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
… replay errors or is aborted (facebook#27386)

Based on facebook#27385.

When we error or abort during replay, that doesn't actually error the
component that errored because that has already rendered. The error only
affects any child that is not yet completed. Therefore the error kind of
gets thrown at the resumable point.

The resumable point might be a hole in the replay path, in which case
throwing there errors the parent boundary just the same as if the replay
component errored. If the hole is inside a deeper Suspense boundary
though, then it's that Suspense boundary that gets client rendered. I.e.
the child boundary. We can still finish any siblings.

In the shell all resumable points are inside a boundary since we must
have finished the shell. Therefore if you error in the root, we just
simply just turn all incomplete boundaries into client renders.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
… replay errors or is aborted (#27386)

Based on #27385.

When we error or abort during replay, that doesn't actually error the
component that errored because that has already rendered. The error only
affects any child that is not yet completed. Therefore the error kind of
gets thrown at the resumable point.

The resumable point might be a hole in the replay path, in which case
throwing there errors the parent boundary just the same as if the replay
component errored. If the hole is inside a deeper Suspense boundary
though, then it's that Suspense boundary that gets client rendered. I.e.
the child boundary. We can still finish any siblings.

In the shell all resumable points are inside a boundary since we must
have finished the shell. Therefore if you error in the root, we just
simply just turn all incomplete boundaries into client renders.

DiffTrain build for commit 925c66a.
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