-
Notifications
You must be signed in to change notification settings - Fork 49.9k
[Flight] Exclude RSC Stream if the stream resolves in a task #34838
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
Conversation
| wakeChunk(resolveListeners, handler.value, initializedChunk); | ||
| } else { | ||
| if (__DEV__) { | ||
| moveDebugInfoFromChunkToInnerValue(initializedChunk, handler.value); |
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.
This PR surfaced another bug.
It turns out that resolveChunkDebugInfo adds a .then() listener which forces the wakeChunk pass to be taken. If that doesn't happen, then wakeChunk isn't called.
We forgot that we need to call moveDebugInfoFromChunkToInnerValue everywhere we initialize but don't call wakeChunk. We could potentially just move this out and all it next to all these instead of inside wakeChunk.
| return null; | ||
| } | ||
| visited.add(node); | ||
| if (node.end >= 0 && node.end <= request.timeOrigin) { |
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.
Another drive-by fix somewhat related. Most of the time this is covered by the PROMISE_NODE path but it really applies to all of these so we can just apply it universally.
Ignore any path that completed before the render started.
This is a bit more optimized because it doesn't create an intermediate unnecessary array.
…eners wakeChunk doesn't get called if we initialize a value without any listeners.
09e1648 to
8cdc1ea
Compare
Stacked on #34836.
Normally we consider microtasks, next ticks and setImmediates not as I/O to be tracked by debug info. We also exclude things that finished loading before we started rendering. That's consider cached.
This avoids emitting the "RSC stream" debug info if the stream finished within a
setTimeout. I.e. it was likely cached and so it wouldn't add any additional suspending.Unfortunately, this will often happen if you use the
createFromReadableStreamAPI to consumeresponse.bodybecause by the time you get there you'll likely have at least received part of the body already.