-
Notifications
You must be signed in to change notification settings - Fork 47k
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
[Flight][Static] Implement halting a prerender behind enableHalt #30705
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
f953493
to
398aa80
Compare
398aa80
to
ab5b304
Compare
ab5b304
to
43e51e0
Compare
43e51e0
to
1227997
Compare
1227997
to
c93a575
Compare
c93a575
to
9e45ae9
Compare
ba510dd
to
5a91fee
Compare
There was a problem hiding this 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).
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 |
5a91fee
to
c891489
Compare
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
c891489
to
29572d9
Compare
@@ -1798,6 +1818,7 @@ function serializeLazyID(id: number): string { | |||
function serializeInfinitePromise(): string { | |||
return '$@'; | |||
} | |||
const reusableInfinitePromiseModel = stringify(serializeInfinitePromise()); |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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.
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)
**breaking change for canary users: Bumps peer dependency of React from `19.0.0-rc-1eaccd82-20240816` to `19.0.0-rc-eb3ad065-20240822`** No changes required in Next.js it seems. [diff facebook/react@1eaccd82...eb3ad065](facebook/react@1eaccd8...eb3ad06) <details> <summary>React upstream changes</summary> - facebook/react#30761 - facebook/react#30779 - facebook/react#30775 - facebook/react#30770 - facebook/react#30756 - facebook/react#30755 - facebook/react#30768 - facebook/react#30760 - facebook/react#30732 - facebook/react#30757 - facebook/react#30750 - facebook/react#30751 - facebook/react#30753 - facebook/react#30740 - facebook/react#30748 - facebook/react#30746 - facebook/react#30747 - facebook/react#30731 - facebook/react#30725 - facebook/react#30741 - facebook/react#30730 - facebook/react#30726 - facebook/react#30717 - facebook/react#30729 - facebook/react#30721 - facebook/react#30720 - facebook/react#30705 </details>
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