From ea441f6a6a797026cca80a92c9f96e21108d6f77 Mon Sep 17 00:00:00 2001
From: Josh Story
Date: Tue, 9 Apr 2024 14:24:29 -0700
Subject: [PATCH] [Fizz] hoistables should never flush before the preamble
Hoistables should never flush before the preamble however there is a surprisingly easy way to trigger this to happen by suspending in the shell of the app. This change modifies the flushing behavior to not emit any hoistables before the preamble has written. It accomplishes this by aborting the flush early if there are any pending root tasks remaining. It's unforunate we need this extra condition but it's essential that we don't emit anything before the preamble and at the moment I don't see a way to do that without introducing a new condition.
---
.../src/__tests__/ReactDOMFloat-test.js | 45 +++++++++++++++++++
packages/react-server/src/ReactFizzServer.js | 19 ++++----
2 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
index c71af838e65dd..0aaaf1a00a71c 100644
--- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
@@ -5002,6 +5002,51 @@ body {
);
});
+ it('should never flush hoistables before the preamble', async () => {
+ let resolve;
+ const promise = new Promise(res => {
+ resolve = res;
+ });
+
+ function App() {
+ ReactDOM.preinit('foo', {as: 'script'});
+ React.use(promise);
+ return (
+
+ hello
+
+ );
+ }
+
+ await act(() => {
+ renderToPipeableStream().pipe(writable);
+ });
+
+ // we assert the default JSDOM still in tact
+ expect(getMeaningfulChildren(document)).toEqual(
+
+
+
+
+
+ ,
+ );
+
+ await act(() => {
+ resolve();
+ });
+
+ // we assert the DOM was replaced entirely because we streamed an opening html tag
+ expect(getMeaningfulChildren(document)).toEqual(
+
+
+
+
+ hello
+ ,
+ );
+ });
+
describe('ReactDOM.prefetchDNS(href)', () => {
it('creates a dns-prefetch resource when called', async () => {
function App({url}) {
diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js
index 223d63a4c1084..7ba6f334e759a 100644
--- a/packages/react-server/src/ReactFizzServer.js
+++ b/packages/react-server/src/ReactFizzServer.js
@@ -4057,22 +4057,25 @@ function flushCompletedQueues(
// that item fully and then yield. At that point we remove the already completed
// items up until the point we completed them.
+ if (request.pendingRootTasks > 0) {
+ // When there are pending root tasks we don't want to flush anything
+ return;
+ }
+
let i;
const completedRootSegment = request.completedRootSegment;
if (completedRootSegment !== null) {
if (completedRootSegment.status === POSTPONED) {
// We postponed the root, so we write nothing.
return;
- } else if (request.pendingRootTasks === 0) {
- flushPreamble(request, destination, completedRootSegment);
- flushSegment(request, destination, completedRootSegment, null);
- request.completedRootSegment = null;
- writeCompletedRoot(destination, request.renderState);
- } else {
- // We haven't flushed the root yet so we don't need to check any other branches further down
- return;
}
+
+ flushPreamble(request, destination, completedRootSegment);
+ flushSegment(request, destination, completedRootSegment, null);
+ request.completedRootSegment = null;
+ writeCompletedRoot(destination, request.renderState);
}
+
writeHoistables(destination, request.resumableState, request.renderState);
// We emit client rendering instructions for already emitted boundaries first.
// This is so that we can signal to the client to start client rendering them as