Skip to content

Commit 5a04619

Browse files
authored
[Flight] Properly close stream when no chunks need to be written after prerender (facebook#33982)
There is an edge case when prerendering where if you have nothing to write you can end up in a state where the prerender is in status closed before you can provide a destination. In this case the destination is never closed becuase it assumes it already would have been. This condition can happen now because of the introduction of the deubg stream. Before this a request would never entere closed status if there was no active destination. When a destination was added it would perform a flush and possibly close the stream. Now, it is possible to flush without a destination because you might have debug chunks to stream and you can end up closing the stream independent of an active destination. There are a number of ways we can solve this but the one that seems to adhere best to the original design is to only set the status to CLOSED when a destination is active. This means that if you don't have an active destination when the pendingChunks count hits zero it will not enter CLOSED status until you startFlowing.
1 parent 129aa85 commit 5a04619

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,4 +863,46 @@ describe('ReactFlightDOMNode', () => {
863863
expect(ownerStack).toBeNull();
864864
}
865865
});
866+
867+
// @gate experimental
868+
// @gate enableHalt
869+
it('can handle an empty prelude when prerendering', async () => {
870+
function App() {
871+
return null;
872+
}
873+
874+
const serverAbortController = new AbortController();
875+
serverAbortController.abort();
876+
const errors = [];
877+
const {pendingResult} = await serverAct(async () => {
878+
// destructure trick to avoid the act scope from awaiting the returned value
879+
return {
880+
pendingResult: ReactServerDOMStaticServer.unstable_prerender(
881+
ReactServer.createElement(App, null),
882+
webpackMap,
883+
{
884+
signal: serverAbortController.signal,
885+
onError(error) {
886+
errors.push(error);
887+
},
888+
},
889+
),
890+
};
891+
});
892+
893+
expect(errors).toEqual([]);
894+
895+
const {prelude} = await pendingResult;
896+
897+
const reader = prelude.getReader();
898+
while (true) {
899+
const {done} = await reader.read();
900+
if (done) {
901+
break;
902+
}
903+
}
904+
905+
// We don't really have an assertion other than to make sure
906+
// the stream doesn't hang.
907+
});
866908
});

packages/react-server/src/ReactFlightServer.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5762,6 +5762,7 @@ function flushCompletedChunks(request: Request): void {
57625762
// TODO: If this destination is not currently flowing we'll not close it when it resumes flowing.
57635763
// We should keep a separate status for this.
57645764
if (request.destination !== null) {
5765+
request.status = CLOSED;
57655766
close(request.destination);
57665767
request.destination = null;
57675768
}
@@ -5779,8 +5780,8 @@ function flushCompletedChunks(request: Request): void {
57795780
);
57805781
request.cacheController.abort(abortReason);
57815782
}
5782-
request.status = CLOSED;
57835783
if (request.destination !== null) {
5784+
request.status = CLOSED;
57845785
close(request.destination);
57855786
request.destination = null;
57865787
}

0 commit comments

Comments
 (0)