Skip to content

[Flight] Include I/O not awaited in user space #33715

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 7, 2025
+393 −251
Merged

[Flight] Include I/O not awaited in user space #33715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d17ffc6
Include I/O not awaited in user space
sebmarkbage Jul 6, 2025
28b7f3e
Use a "previous" IO node as the awaited I/O if the "next" doesn't hav…
sebmarkbage Jul 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions packages/react-server/src/ReactFlightServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2052,20 +2052,17 @@ function visitAsyncNode(
return null;
}
visited.add(node);
let previousIONode = null;
// First visit anything that blocked this sequence to start in the first place.
if (node.previous !== null && node.end > request.timeOrigin) {
// We ignore the returned io nodes here because if it wasn't awaited in user space,
// then we don't log it. It also means that it can just have been part of a previous
// component's render.
// TODO: This means that some I/O can get lost that was still blocking the sequence.
const ioNode = visitAsyncNode(
previousIONode = visitAsyncNode(
request,
task,
node.previous,
visited,
cutOff,
);
if (ioNode === undefined) {
if (previousIONode === undefined) {
// Undefined is used as a signal that we found a suitable aborted node and we don't have to find
// further aborted nodes.
return undefined;
Expand All @@ -2076,17 +2073,17 @@ function visitAsyncNode(
return node;
}
case UNRESOLVED_PROMISE_NODE: {
return null;
return previousIONode;
}
case PROMISE_NODE: {
if (node.end <= request.timeOrigin) {
// This was already resolved when we started this render. It must have been either something
// that's part of a start up sequence or externally cached data. We exclude that information.
// The technique for debugging the effects of uncached data on the render is to simply uncache it.
return null;
return previousIONode;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment above makes we wonder whether this change may affect cached I/O. At least in the Flight fixture it doesn't seem to cause an issue with the cached third-party stream though.

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 previousIONode has its own check if it finished before this sequence so it’ll also be null.

We don’t really expect the future node to have finished before the previous. But for consistency I return it here.

}
const awaited = node.awaited;
let match = null;
let match: void | null | PromiseNode | IONode = previousIONode;
if (awaited !== null) {
const ioNode = visitAsyncNode(request, task, awaited, visited, cutOff);
if (ioNode === undefined) {
Expand Down Expand Up @@ -2134,11 +2131,11 @@ function visitAsyncNode(
return match;
}
case UNRESOLVED_AWAIT_NODE: {
return null;
return previousIONode;
}
case AWAIT_NODE: {
const awaited = node.awaited;
let match = null;
let match: void | null | PromiseNode | IONode = previousIONode;
if (awaited !== null) {
const ioNode = visitAsyncNode(request, task, awaited, visited, cutOff);
if (ioNode === undefined) {
Expand Down
Loading
Loading