Skip to content

Conversation

@gnoff
Copy link
Collaborator

@gnoff gnoff commented Jul 31, 2024

When aborting with a postpone value in Fizz if any tasks are still pending in the root while prerendering the prerender will fatally error. This is different from postponing imperatively in a root task and really the semantics should be the same. This change updates React to treat an abort with a postpone value as a postponed root rather than a fatal error.

@vercel
Copy link

vercel bot commented Jul 31, 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 Jul 31, 2024 3:20pm

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jul 31, 2024
Comment on lines +3799 to +3805
if (trackedPostpones !== null && segment !== null) {
// We are prerendering. We don't want to fatal when the shell postpones
// we just need to mark it as postponed.
logPostpone(request, postponeInstance.message, errorInfo, null);
trackPostpone(request, trackedPostpones, task, segment);
finishedTask(request, null, segment);
} else {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The factoring here isn't great but I don't want to completely refactor this code. I have to recheck wether the segment actually exists to mark it as postponable but if we're tracking postpones it would definitely exist. But mabye I should

@react-sizebot
Copy link

react-sizebot commented Jul 31, 2024

Comparing: 2b00018...2afe9c6

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 = 1.83 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 500.39 kB 500.39 kB = 89.78 kB 89.78 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 507.52 kB 507.52 kB = 90.95 kB 90.95 kB
facebook-www/ReactDOM-prod.classic.js = 595.41 kB 595.41 kB = 105.58 kB 105.58 kB
facebook-www/ReactDOM-prod.modern.js = 571.71 kB 571.71 kB = 101.78 kB 101.78 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.49% 117.60 kB 118.18 kB +0.80% 21.21 kB 21.38 kB
oss-experimental/react-server/cjs/react-server.development.js +0.39% 171.68 kB 172.34 kB +0.46% 31.01 kB 31.15 kB
oss-experimental/react-html/cjs/react-html.production.js +0.28% 202.46 kB 203.03 kB +0.36% 38.09 kB 38.23 kB
oss-stable-rc/react-server/cjs/react-server.production.js +0.26% 107.19 kB 107.47 kB +0.45% 19.67 kB 19.76 kB
oss-stable-semver/react-server/cjs/react-server.production.js +0.26% 107.19 kB 107.47 kB +0.45% 19.67 kB 19.76 kB
oss-stable/react-server/cjs/react-server.production.js +0.26% 107.19 kB 107.47 kB +0.45% 19.67 kB 19.76 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.js +0.26% 214.38 kB 214.94 kB +0.34% 38.96 kB 39.09 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.js +0.26% 219.33 kB 219.90 kB +0.34% 40.81 kB 40.95 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.js +0.26% 220.05 kB 220.62 kB +0.34% 40.22 kB 40.36 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.js +0.23% 241.25 kB 241.81 kB +0.31% 43.47 kB 43.61 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.js +0.23% 239.88 kB 240.43 kB +0.34% 42.22 kB 42.36 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.production.js +0.23% 245.31 kB 245.88 kB +0.30% 44.21 kB 44.34 kB

Generated by 🚫 dangerJS against 58fe77a

);
});

// @gate experimental
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be gated on enablePostpone I guess.

When aborting with a postpone value in Fizz if any tasks are still pending in the root while prerendering the prerender will fatally error. This is different from postponing imperatively in a root task and really the semantics should be the same. This change updatse React to treat an abort with a postpone value as a postponed root rather than a fatal error.
@gnoff gnoff force-pushed the prerender-abort branch from 63f552a to 58fe77a Compare July 31, 2024 15:18
@gnoff gnoff merged commit a7d1240 into facebook:main Jul 31, 2024
@gnoff gnoff deleted the prerender-abort branch July 31, 2024 15:33
github-actions bot pushed a commit that referenced this pull request Jul 31, 2024
When aborting with a postpone value in Fizz if any tasks are still
pending in the root while prerendering the prerender will fatally error.
This is different from postponing imperatively in a root task and really
the semantics should be the same. This change updates React to treat an
abort with a postpone value as a postponed root rather than a fatal
error.

DiffTrain build for [a7d1240](a7d1240)
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