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

[Flight][Static] Implement halting a prerender behind enableHalt #30705

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Aug 15, 2024

enableHalt turns on a mode for flight prerenders where aborts are treated like infinitely stalled outcomes while still completing the prerender. For regular tasks we simply serialize the slot as a promise that never settles. For ReadableStream, Blob, and Async Iterators we just never advance the serialization so they remain unfinished when consumed on the client.

When enableHalt is turned on aborts of prerenders will halt rather than error. The abort reason is forwarded to the upstream produces of the aforementioned async iterators, blobs, and ReadableStreams. In the future if we expose a signal that you can consume from within a render to cancel additional work the abort reason will also be forwarded there

Copy link

vercel bot commented Aug 15, 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 16, 2024 8:58pm

@react-sizebot
Copy link

react-sizebot commented Aug 15, 2024

Comparing: 1eaccd8...392b9c8

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.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
oss-experimental/react-server/cjs/react-server-flight.production.js +3.38% 60.96 kB 63.02 kB +1.88% 12.20 kB 12.43 kB
oss-experimental/react-server/cjs/react-server-flight.development.js +2.37% 101.32 kB 103.72 kB +1.54% 18.60 kB 18.88 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-server.node.production.js +2.04% 92.82 kB 94.71 kB +1.09% 19.19 kB 19.40 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-flight.production.js +3.38% 60.96 kB 63.02 kB +1.88% 12.20 kB 12.43 kB
oss-experimental/react-server/cjs/react-server-flight.development.js +2.37% 101.32 kB 103.72 kB +1.54% 18.60 kB 18.88 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-server.node.production.js +2.04% 92.82 kB 94.71 kB +1.09% 19.19 kB 19.40 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.browser.production.js +1.97% 95.99 kB 97.89 kB +1.37% 19.48 kB 19.75 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.production.js +1.97% 96.31 kB 98.20 kB +1.37% 19.57 kB 19.84 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.production.js +1.96% 96.41 kB 98.31 kB +1.41% 19.59 kB 19.87 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.edge.production.js +1.96% 96.44 kB 98.34 kB +1.41% 19.59 kB 19.87 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.production.js +1.92% 98.47 kB 100.37 kB +1.04% 20.07 kB 20.28 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.unbundled.production.js +1.92% 98.49 kB 100.39 kB +1.04% 20.07 kB 20.28 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.production.js +1.90% 99.43 kB 101.32 kB +1.37% 20.28 kB 20.55 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.production.js +1.90% 99.46 kB 101.36 kB +1.37% 20.27 kB 20.55 kB
oss-stable-rc/react-server/cjs/react-server-flight.production.js +1.78% 56.88 kB 57.89 kB +0.98% 11.48 kB 11.59 kB
oss-stable-semver/react-server/cjs/react-server-flight.production.js +1.78% 56.88 kB 57.89 kB +0.98% 11.48 kB 11.59 kB
oss-stable/react-server/cjs/react-server-flight.production.js +1.78% 56.88 kB 57.89 kB +0.98% 11.48 kB 11.59 kB
oss-experimental/react-server-dom-esm/cjs/react-server-dom-esm-server.node.development.js +1.66% 143.76 kB 146.14 kB +1.23% 26.48 kB 26.81 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.browser.development.js +1.63% 146.67 kB 149.06 kB +1.17% 26.87 kB 27.18 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.browser.development.js +1.62% 147.29 kB 149.67 kB +1.15% 27.05 kB 27.36 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.edge.development.js +1.60% 149.51 kB 151.90 kB +1.19% 27.24 kB 27.56 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.edge.development.js +1.59% 149.66 kB 152.05 kB +1.19% 27.30 kB 27.63 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.unbundled.development.js +1.59% 150.12 kB 152.51 kB +1.14% 27.47 kB 27.78 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.unbundled.development.js +1.59% 150.28 kB 152.67 kB +1.14% 27.53 kB 27.84 kB
oss-experimental/react-server-dom-turbopack/cjs/react-server-dom-turbopack-server.node.development.js +1.58% 151.24 kB 153.62 kB +1.14% 27.73 kB 28.05 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-server.node.development.js +1.58% 151.38 kB 153.77 kB +1.14% 27.80 kB 28.12 kB
oss-stable-rc/react-server/cjs/react-server-flight.development.js +1.23% 86.79 kB 87.85 kB +0.71% 16.10 kB 16.22 kB
oss-stable-semver/react-server/cjs/react-server-flight.development.js +1.23% 86.79 kB 87.85 kB +0.71% 16.10 kB 16.22 kB
oss-stable/react-server/cjs/react-server-flight.development.js +1.23% 86.79 kB 87.85 kB +0.71% 16.10 kB 16.22 kB
oss-experimental/react-markup/cjs/react-markup.react-server.production.js +0.34% 300.55 kB 301.58 kB +0.26% 56.84 kB 56.98 kB
oss-experimental/react-markup/cjs/react-markup.react-server.development.js +0.30% 497.97 kB 499.45 kB +0.33% 89.64 kB 89.93 kB

Generated by 🚫 dangerJS against 29572d9

@gnoff gnoff force-pushed the prerender-infinite-promise-on-abort branch 2 times, most recently from f953493 to 398aa80 Compare August 15, 2024 05:37
@gnoff gnoff force-pushed the prerender-infinite-promise-on-abort branch from 398aa80 to ab5b304 Compare August 15, 2024 22:02
@gnoff gnoff force-pushed the prerender-infinite-promise-on-abort branch from ab5b304 to 43e51e0 Compare August 16, 2024 05:47
@gnoff gnoff force-pushed the prerender-infinite-promise-on-abort branch from 43e51e0 to 1227997 Compare August 16, 2024 05:51
@gnoff gnoff marked this pull request as ready for review August 16, 2024 05:53
@gnoff gnoff force-pushed the prerender-infinite-promise-on-abort branch from 1227997 to c93a575 Compare August 16, 2024 06:10
@gnoff gnoff changed the title [Flight][Static] When prerendering serialize infinite promise when aborting with no reason [Flight][Static] Implement halting a prerender behind enableHalt Aug 16, 2024
@gnoff gnoff force-pushed the prerender-infinite-promise-on-abort branch from c93a575 to 9e45ae9 Compare August 16, 2024 06:22
@gnoff gnoff force-pushed the prerender-infinite-promise-on-abort branch 3 times, most recently from ba510dd to 5a91fee Compare August 16, 2024 06:35
@gnoff gnoff requested a review from sebmarkbage August 16, 2024 06:39
Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

It feels a little unnecessary/hacky to introduce another object kind with duck-typing especially if postpone is going away and then ideally thrown promises go away too.

How about instead we store the reason on the request like we do for fatalError and use it if we're in the halting status. It can even be the same slot with a different status (we already abuse it for aborting in the same way).

@gnoff
Copy link
Collaborator Author

gnoff commented Aug 16, 2024

@sebmarkbage

How about instead we store the reason on the request like we do for fatalError and use it if we're in the halting status.

We are currently using that slot for the ref id. I could instead just always emit an infinite promise into those slots instead though rather than referring to the a shared one since it's basically the same size

enableHalt turns on a mode for flight prerenders where aborts are treated like infinitely stalled outcomes while still completing the prerender. For regular tasks we simply serialize the slot as a promise that never settles. For ReadableStream, Blob, and Async Iterators we just never advance the serialization so they remain unfinished when consumed on the client.

When enableHalt is turned on aborts of prerenders will halt rather than error. The abort reason is forwarded to the upstream produces of the aforementioned async iterators, blobs, and ReadableStreams. In the future if we expose a signal that you can consume from within a render to cancel additional work the abort reason will also be forwarded there
@gnoff gnoff force-pushed the prerender-infinite-promise-on-abort branch from c891489 to 29572d9 Compare August 16, 2024 20:56
@gnoff gnoff merged commit 7b41cdc into facebook:main Aug 16, 2024
185 checks passed
@gnoff gnoff deleted the prerender-infinite-promise-on-abort branch August 16, 2024 21:22
@@ -1798,6 +1818,7 @@ function serializeLazyID(id: number): string {
function serializeInfinitePromise(): string {
return '$@';
}
const reusableInfinitePromiseModel = stringify(serializeInfinitePromise());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this get inlined btw? Might as well just hard code the string to ensure it does. There's not really any purpose in having it be hoisted value since it'll be interned anyway.

@@ -2202,6 +2232,9 @@ function renderModel(

if (request.status === ABORTING) {
task.status = ABORTED;
if (enableHalt && request.fatalError === haltSymbol) {
return serializeInfinitePromise();
Copy link
Collaborator

@sebmarkbage sebmarkbage Aug 17, 2024

Choose a reason for hiding this comment

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

So I don't think this is actually right. Because this is saying that this value will be a Promise but if this wasn't already a Promise that's incorrect. It's not the same as postpone which gets serialized as an error. What you want here is more like serializeByValueID to an ID that will never resolve.

The only time you can use serializeInfinitePromise() is when you're actually serializing a Promise/Thenable.

gnoff added a commit that referenced this pull request Aug 19, 2024
using infinitely suspending promises isn't right because this will parse
as a promise which is only appropriate if the value we're halting at is
a promise. Instead we need to have a special marker type that says this
reference will never resolve. Additionally flight client needs to not
error any halted references when the stream closes because they will
otherwise appear as an error

addresses:
#30705 (comment)
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