Skip to content
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

Jump to bottom

[Fizz] hoistables should never flush before the preamble #28802

Merged
merged 1 commit into from
Apr 11, 2024
+64 −9
Merged

[Fizz] hoistables should never flush before the preamble #28802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 8 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6926,7 +6926,14 @@ describe('ReactDOMFizzServer', () => {
expect(getVisibleChildren(container)).toEqual(
<div>
{'Loading1'}
{'Hello'}
{/*
This used to show "Hello" in this slot because the boundary was able to be flushed
early but we now prevent flushing while pendingRootTasks is not zero. This is how Edge
would work anyway because you don't get the stream until the root is unblocked on a resume
so Node now aligns with edge bevavior
{'Hello'}
*/}
{'Loading2'}
{'Loading3'}
</div>,
);
Expand Down
45 changes: 45 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5034,6 +5034,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 (
<html>
<body>hello</body>
</html>
);
}

await act(() => {
renderToPipeableStream(<App />).pipe(writable);
});

// we assert the default JSDOM still in tact
expect(getMeaningfulChildren(document)).toEqual(
<html>
<head />
<body>
<div id="container" />
</body>
</html>,
);

await act(() => {
resolve();
});

// we assert the DOM was replaced entirely because we streamed an opening html tag
expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<script async="" src="foo" />
</head>
<body>hello</body>
</html>,
);
});

describe('ReactDOM.prefetchDNS(href)', () => {
it('creates a dns-prefetch resource when called', async () => {
function App({url}) {
Expand Down
19 changes: 11 additions & 8 deletions packages/react-server/src/ReactFizzServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down