Add Postpone API #27238
Merged
Add Postpone API #27238
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Serialize as P. In dev we include reason and stack. Emit onPostpone event if it happens with the reason.
Deserialize into the same shape as postpone(). Could actually call it potentially.
This just uses a fake infinite promise as a short cut. We could be more clever in the code here and not create any listeners at all.
We just don't use native classes for this and brand check with $$typeof.
facebook-github-bot
added
CLA Signed
React Core Team
sebmarkbage
force-pushed
the
postpone
branch
2 times, most recently
from
8c4c2fa to
dbcc12f
Compare
sebmarkbage
force-pushed
the
postpone
branch
4 times, most recently
from
cca7c85 to
6b31297
Compare
This is semantically the same as just throwing an error. It just triggers client rendering. The difference is in how it gets logged. On the server we log to onPostpone instead of onError. On the client this doesn't trigger a recoverable error. It's just silent since it was intentional.
sebmarkbage
commented
Aug 17, 2023
| @@ -2859,27 +2860,32 @@ function updateDehydratedSuspenseComponent( | |||
| // This boundary is in a permanent fallback state. In this case, we'll never | |||
| // get an update and we'll never be able to hydrate the final content. Let's just try the | |||
| // client side render instead. | |||
| let digest, message, stack; | |||
| let digest: ?string; | |||
There was a problem hiding this comment.
I hit a weird Flow error that only repro:ed on CI and not locally. I thought they were supposed to be the same version. It considers it empty even though it's always initialized - unless I annotate.
gnoff
approved these changes
Aug 17, 2023
Comment on lines
1385
to
1388
| } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { | ||
| const postponeInstance: Postpone = (x: any); | ||
| logPostpone(request, postponeInstance.message); | ||
| emitPostponeChunk(request, task.id, postponeInstance); |
There was a problem hiding this comment.
Is the task supposed to be in a new status here? I don't see why it would get retried but if it did it would attempt to resolve the model again.
There was a problem hiding this comment.
yea, that's right and it should probably be removed from the abortable tasks list so it doesn't get aborted later.
Comment on lines
+1718
to
+1719
| // TODO: Figure out a better signal than a magic digest value. | ||
| errorDigest = 'POSTPONE'; |
There was a problem hiding this comment.
It's maybe not much better but we could have non-postpone digest be null or non-empty string and then empty string digest can represent postpones. This means collisions are impossible. The questions is really whether normal errors without a digest or postpones are more common, whichever is should be the one that omits the digest value and the other can be represented by an empty string
There was a problem hiding this comment.
I think postpones should be more common. E.g. triggering intentional client rendering would happen every time. Where as errors really shouldn't happen at all ideally.
There was a problem hiding this comment.
Well, bugs are probably more common but when that happens you're down the path of a bad experience anyway.
| // These errors should never make it into a build so we don't need to encode them in codes.json | ||
| // eslint-disable-next-line react-internal/prod-error-codes | ||
| throw new Error( | ||
| 'resolveErrorProd should never be called in development mode. Use resolveErrorDev instead. This is a bug in React.', |
There was a problem hiding this comment.
error message has wrong method name
| // These errors should never make it into a build so we don't need to encode them in codes.json | ||
| // eslint-disable-next-line react-internal/prod-error-codes | ||
| throw new Error( | ||
| 'resolveErrorDev should never be called in production mode. Use resolveErrorProd instead. This is a bug in React.', |
There was a problem hiding this comment.
fix method name in error message
There was a problem hiding this comment.
I intentionally did this because I hate taking numbers away from the codes.json for these internal things that shouldn't happen and if I just inlined this wouldn't be necessary. :)
There was a problem hiding this comment.
I guess these are not in the codes file so maybe that's fine.
| if (value !== null && typeof value === 'object') { | ||
| if (enablePostpone && value.$$typeof === REACT_POSTPONE_TYPE) { | ||
| // Act as if this is an infinitely suspending promise. | ||
| value = {then: function () {}}; |
There was a problem hiding this comment.
TIL our implementation of thenable does not assume the result is also thenable
github-actions bot
pushed a commit
that referenced
this pull request
Aug 17, 2023
This adds an experimental `unstable_postpone(reason)` API. Currently we don't have a way to model effectively an Infinite Promise. I.e. something that suspends but never resolves. The reason this is useful is because you might have something else that unblocks it later. E.g. by updating in place later, or by client rendering. On the client this works to model as an Infinite Promise (in fact, that's what this implementation does). However, in Fizz and Flight that doesn't work because the stream needs to end at some point. We don't have any way of knowing that we're suspended on infinite promises. It's not enough to tag the promises because you could await those and thus creating new promises. The only way we really have to signal this through a series of indirections like async functions, is by throwing. It's not 100% safe because these values can be caught but it's the best we can do. Effectively `postpone(reason)` behaves like a built-in [Catch Boundary](#26854). It's like `raise(Postpone, reason)` except it's built-in so it needs to be able to be encoded and caught by Suspense boundaries. In Flight and Fizz these behave pretty much the same as errors. Flight just forwards it to retrigger on the client. In Fizz they just trigger client rendering which itself might just postpone again or fill in the value. The difference is how they get logged. In Flight and Fizz they log to `onPostpone(reason)` instead of `onError(error)`. This log is meant to help find deopts on the server like finding places where you fall back to client rendering. The reason that you pass in is for that purpose to help the reason for any deopts. I do track the stack trace in DEV but I don't currently expose it to `onPostpone`. This seems like a limitation. It might be better to expose the Postpone object which is an Error object but that's more of an implementation detail. I could also pass it as a second argument. On the client after hydration they don't get passed to `onRecoverableError`. There's no global `onPostpone` API to capture postponed things on the client just like there's no `onError`. At that point it's just assumed to be intentional. It doesn't have any `digest` or reason passed to the client since it's not logged. There are some hacky solutions that currently just tries to reuse as much of the existing code as possible but should be more properly implemented. - Fiber is currently just converting it to a fake Promise object so that it behaves like an infinite Promise. - Fizz is encoding the magic digest string `"POSTPONE"` in the HTML so we know to ignore it but it should probably just be something neater that doesn't share namespace with digests. Next I plan on using this in the `/static` entry points for additional features. Why "postpone"? It's basically a synonym to "defer" but we plan on using "defer" for other purposes and it's overloaded anyway. DiffTrain build for [ac1a16c](ac1a16c)
kodiakhq bot
pushed a commit
to vercel/next.js
that referenced
this pull request
Aug 22, 2023
### React upstream changes - facebook/react#27265 - facebook/react#27259 - facebook/react#27264 - facebook/react#27257 - facebook/react#27258 - facebook/react#27187 - facebook/react#27243 - facebook/react#27205 - facebook/react#27220 - facebook/react#27238 - facebook/react#27234 - facebook/react#27224 - facebook/react#27223 - facebook/react#27222 This will help unblock #53906
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
This adds an experimental `unstable_postpone(reason)` API. Currently we don't have a way to model effectively an Infinite Promise. I.e. something that suspends but never resolves. The reason this is useful is because you might have something else that unblocks it later. E.g. by updating in place later, or by client rendering. On the client this works to model as an Infinite Promise (in fact, that's what this implementation does). However, in Fizz and Flight that doesn't work because the stream needs to end at some point. We don't have any way of knowing that we're suspended on infinite promises. It's not enough to tag the promises because you could await those and thus creating new promises. The only way we really have to signal this through a series of indirections like async functions, is by throwing. It's not 100% safe because these values can be caught but it's the best we can do. Effectively `postpone(reason)` behaves like a built-in [Catch Boundary](facebook#26854). It's like `raise(Postpone, reason)` except it's built-in so it needs to be able to be encoded and caught by Suspense boundaries. In Flight and Fizz these behave pretty much the same as errors. Flight just forwards it to retrigger on the client. In Fizz they just trigger client rendering which itself might just postpone again or fill in the value. The difference is how they get logged. In Flight and Fizz they log to `onPostpone(reason)` instead of `onError(error)`. This log is meant to help find deopts on the server like finding places where you fall back to client rendering. The reason that you pass in is for that purpose to help the reason for any deopts. I do track the stack trace in DEV but I don't currently expose it to `onPostpone`. This seems like a limitation. It might be better to expose the Postpone object which is an Error object but that's more of an implementation detail. I could also pass it as a second argument. On the client after hydration they don't get passed to `onRecoverableError`. There's no global `onPostpone` API to capture postponed things on the client just like there's no `onError`. At that point it's just assumed to be intentional. It doesn't have any `digest` or reason passed to the client since it's not logged. There are some hacky solutions that currently just tries to reuse as much of the existing code as possible but should be more properly implemented. - Fiber is currently just converting it to a fake Promise object so that it behaves like an infinite Promise. - Fizz is encoding the magic digest string `"POSTPONE"` in the HTML so we know to ignore it but it should probably just be something neater that doesn't share namespace with digests. Next I plan on using this in the `/static` entry points for additional features. Why "postpone"? It's basically a synonym to "defer" but we plan on using "defer" for other purposes and it's overloaded anyway.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
This adds an experimental `unstable_postpone(reason)` API. Currently we don't have a way to model effectively an Infinite Promise. I.e. something that suspends but never resolves. The reason this is useful is because you might have something else that unblocks it later. E.g. by updating in place later, or by client rendering. On the client this works to model as an Infinite Promise (in fact, that's what this implementation does). However, in Fizz and Flight that doesn't work because the stream needs to end at some point. We don't have any way of knowing that we're suspended on infinite promises. It's not enough to tag the promises because you could await those and thus creating new promises. The only way we really have to signal this through a series of indirections like async functions, is by throwing. It's not 100% safe because these values can be caught but it's the best we can do. Effectively `postpone(reason)` behaves like a built-in [Catch Boundary](#26854). It's like `raise(Postpone, reason)` except it's built-in so it needs to be able to be encoded and caught by Suspense boundaries. In Flight and Fizz these behave pretty much the same as errors. Flight just forwards it to retrigger on the client. In Fizz they just trigger client rendering which itself might just postpone again or fill in the value. The difference is how they get logged. In Flight and Fizz they log to `onPostpone(reason)` instead of `onError(error)`. This log is meant to help find deopts on the server like finding places where you fall back to client rendering. The reason that you pass in is for that purpose to help the reason for any deopts. I do track the stack trace in DEV but I don't currently expose it to `onPostpone`. This seems like a limitation. It might be better to expose the Postpone object which is an Error object but that's more of an implementation detail. I could also pass it as a second argument. On the client after hydration they don't get passed to `onRecoverableError`. There's no global `onPostpone` API to capture postponed things on the client just like there's no `onError`. At that point it's just assumed to be intentional. It doesn't have any `digest` or reason passed to the client since it's not logged. There are some hacky solutions that currently just tries to reuse as much of the existing code as possible but should be more properly implemented. - Fiber is currently just converting it to a fake Promise object so that it behaves like an infinite Promise. - Fizz is encoding the magic digest string `"POSTPONE"` in the HTML so we know to ignore it but it should probably just be something neater that doesn't share namespace with digests. Next I plan on using this in the `/static` entry points for additional features. Why "postpone"? It's basically a synonym to "defer" but we plan on using "defer" for other purposes and it's overloaded anyway. DiffTrain build for commit ac1a16c.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds an experimental
unstable_postpone(reason)API.Currently we don't have a way to model effectively an Infinite Promise. I.e. something that suspends but never resolves. The reason this is useful is because you might have something else that unblocks it later. E.g. by updating in place later, or by client rendering.
On the client this works to model as an Infinite Promise (in fact, that's what this implementation does). However, in Fizz and Flight that doesn't work because the stream needs to end at some point. We don't have any way of knowing that we're suspended on infinite promises. It's not enough to tag the promises because you could await those and thus creating new promises. The only way we really have to signal this through a series of indirections like async functions, is by throwing. It's not 100% safe because these values can be caught but it's the best we can do.
Effectively
postpone(reason)behaves like a built-in Catch Boundary. It's likeraise(Postpone, reason)except it's built-in so it needs to be able to be encoded and caught by Suspense boundaries.In Flight and Fizz these behave pretty much the same as errors. Flight just forwards it to retrigger on the client. In Fizz they just trigger client rendering which itself might just postpone again or fill in the value. The difference is how they get logged.
In Flight and Fizz they log to
onPostpone(reason)instead ofonError(error). This log is meant to help find deopts on the server like finding places where you fall back to client rendering. The reason that you pass in is for that purpose to help the reason for any deopts.I do track the stack trace in DEV but I don't currently expose it to
onPostpone. This seems like a limitation. It might be better to expose the Postpone object which is an Error object but that's more of an implementation detail. I could also pass it as a second argument.On the client after hydration they don't get passed to
onRecoverableError. There's no globalonPostponeAPI to capture postponed things on the client just like there's noonError. At that point it's just assumed to be intentional. It doesn't have anydigestor reason passed to the client since it's not logged.There are some hacky solutions that currently just tries to reuse as much of the existing code as possible but should be more properly implemented.
"POSTPONE"in the HTML so we know to ignore it but it should probably just be something neater that doesn't share namespace with digests.Next I plan on using this in the
/staticentry points for additional features.Why "postpone"? It's basically a synonym to "defer" but we plan on using "defer" for other purposes and it's overloaded anyway.