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] track postpones when aborting boundaries with a postpone #30751

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Aug 20, 2024

When aborting with a postpone value boundaries are put into client rendered mode even during prerenders. This doesn't follow the postpoen semantics of the rest of fizz where during a prerender a postpone is tracked and it will leave holes in tracked postpone state that can be resumed. This change updates this behavior to match the postpones semantics between aborts and imperative postpones.

Copy link

vercel bot commented Aug 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 20, 2024 3:05am

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 20, 2024
When aborting with a postpone value boundaries are put into client rendered mode even during prerenders. This doesn't follow the postpoen semantics of the rest of fizz where during a prerender a postpone is tracked and it will leave holes in tracked postpone state that can be resumed. This change updates this behavior to match the postpones semantics between aborts and imperative postpones.
@react-sizebot
Copy link

react-sizebot commented Aug 20, 2024

Comparing: a960b92...5195ed1

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.05% 1.82 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 500.37 kB 500.37 kB = 89.80 kB 89.80 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 507.50 kB 507.50 kB = 90.96 kB 90.96 kB
facebook-www/ReactDOM-prod.classic.js = 595.24 kB 595.24 kB = 105.55 kB 105.55 kB
facebook-www/ReactDOM-prod.modern.js = 571.54 kB 571.54 kB = 101.75 kB 101.75 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-server/cjs/react-server.production.js +0.40% 118.16 kB 118.63 kB +0.37% 21.35 kB 21.43 kB
oss-experimental/react-server/cjs/react-server.development.js +0.28% 172.31 kB 172.80 kB +0.17% 31.13 kB 31.18 kB
oss-experimental/react-markup/cjs/react-markup.production.js +0.22% 203.01 kB 203.45 kB +0.17% 38.21 kB 38.27 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.js +0.21% 214.93 kB 215.37 kB +0.17% 39.06 kB 39.13 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.js +0.20% 219.89 kB 220.33 kB +0.17% 40.93 kB 40.99 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.js +0.20% 220.60 kB 221.05 kB +0.18% 40.34 kB 40.41 kB

Generated by 🚫 dangerJS against b7f6c17

@@ -3857,7 +3857,6 @@ function abortTask(task: Task, request: Request, error: mixed): void {
} else {
boundary.pendingTasks--;
if (boundary.status !== CLIENT_RENDERED) {
boundary.status = CLIENT_RENDERED;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The point of setting this right away is to avoid a situation where something throws below and we're not in this status.

We really shouldn't throw below though.


// If this boundary was still pending then we haven't already cancelled its fallbacks.
// We'll need to abort the fallbacks, which will also error that parent boundary.
boundary.fallbackAbortableTasks.forEach(fallbackTask =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the unfortunate case where we hadn't finished the fallback yet which should only be the case if it suspends or has too many awaits.

@gnoff gnoff merged commit 2505bf9 into facebook:main Aug 20, 2024
185 checks passed
@gnoff gnoff deleted the fizz-abort-semantics branch August 20, 2024 16:49
github-actions bot pushed a commit that referenced this pull request Aug 20, 2024
When aborting with a postpone value boundaries are put into client
rendered mode even during prerenders. This doesn't follow the postpoen
semantics of the rest of fizz where during a prerender a postpone is
tracked and it will leave holes in tracked postpone state that can be
resumed. This change updates this behavior to match the postpones
semantics between aborts and imperative postpones.

DiffTrain build for [2505bf9](2505bf9)
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.

4 participants