diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index a93c32a947f10..0fd38390628de 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -4518,7 +4518,7 @@ export function writeCompletedRoot( } if (enableFizzBlockingRender) { const preamble = renderState.preamble; - if (preamble.htmlChunks || preamble.headChunks) { + if (!isComplete && (preamble.htmlChunks || preamble.headChunks)) { // If we rendered the whole document, then we emitted a rel="expect" that needs a // matching target. Normally we use one of the bootstrap scripts for this but if // there are none, then we need to emit a tag to complete the shell. diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index b988bd72caff5..57f82ada51da8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3630,9 +3630,6 @@ describe('ReactDOMFizzServer', () => { '' + (gate(flags => flags.shouldUseFizzExternalRuntime) ? '' - : '') + - (gate(flags => flags.enableFizzBlockingRender) - ? '' : ''), ); }); @@ -4566,15 +4563,7 @@ describe('ReactDOMFizzServer', () => { // the html should be as-is expect(document.documentElement.innerHTML).toEqual( - '' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + - '

hello world!

' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + - '', + '

hello world!

', ); }); @@ -6618,14 +6607,7 @@ describe('ReactDOMFizzServer', () => { (gate(flags => flags.shouldUseFizzExternalRuntime) ? '' : '') + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + - '' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + - '', + '', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index bc96750d46038..7f4a48f5da0c0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -71,15 +71,9 @@ describe('ReactDOMFizzServerBrowser', () => { ), ); const result = await readResult(stream); - if (gate(flags => flags.enableFizzBlockingRender)) { - expect(result).toMatchInlineSnapshot( - `"hello world"`, - ); - } else { - expect(result).toMatchInlineSnapshot( - `"hello world"`, - ); - } + expect(result).toMatchInlineSnapshot( + `"hello world"`, + ); }); it('should emit bootstrap script src at the end', async () => { @@ -522,15 +516,7 @@ describe('ReactDOMFizzServerBrowser', () => { const result = await readResult(stream); expect(result).toEqual( - '' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + - 'foobar' + - (gate(flags => flags.enableFizzBlockingRender) - ? '' - : '') + - '', + 'foobar', ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js index 06b3c61ce3ce5..9e0f3c71aec99 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerEdge-test.js @@ -73,15 +73,9 @@ describe('ReactDOMFizzServerEdge', () => { setTimeout(resolve, 1); }); - if (gate(flags => flags.enableFizzBlockingRender)) { - expect(result).toMatchInlineSnapshot( - `"
hello
"`, - ); - } else { - expect(result).toMatchInlineSnapshot( - `"
hello
"`, - ); - } + expect(result).toMatchInlineSnapshot( + `"
hello
"`, + ); }); it('recoverably errors and does not add rel="expect" for large shells', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 98030d43386c9..a99a9868ea9ff 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -113,15 +113,9 @@ describe('ReactDOMFizzServerNode', () => { pipe(writable); }); // with Float, we emit empty heads if they are elided when rendering - if (gate(flags => flags.enableFizzBlockingRender)) { - expect(output.result).toMatchInlineSnapshot( - `"hello world"`, - ); - } else { - expect(output.result).toMatchInlineSnapshot( - `"hello world"`, - ); - } + expect(output.result).toMatchInlineSnapshot( + `"hello world"`, + ); }); it('should emit bootstrap script src at the end', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNodeIssue34966-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNodeIssue34966-test.js new file mode 100644 index 0000000000000..7eda3776a30c3 --- /dev/null +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNodeIssue34966-test.js @@ -0,0 +1,160 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + * @jest-environment node + */ + +'use strict'; + +let Stream; +let React; +let ReactDOMFizzServer; +let Suspense; +let lazy; +let act; + +describe('ReactDOMFizzServerNode Issue 34966', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactDOMFizzServer = require('react-dom/server'); + Stream = require('stream'); + Suspense = React.Suspense; + lazy = React.lazy; + act = require('internal-test-utils').act; + }); + + function getTestWritable() { + const writable = new Stream.PassThrough(); + writable.setEncoding('utf8'); + const output = {result: '', error: undefined}; + writable.on('data', chunk => { + output.result += chunk; + }); + writable.on('error', error => { + output.error = error; + }); + const completed = new Promise(resolve => { + writable.on('finish', () => { + resolve(); + }); + writable.on('error', () => { + resolve(); + }); + }); + return {writable, completed, output}; + } + + it('should not inject streaming scripts in onAllReady with lazy components', async () => { + const Button = () => + React.createElement( + 'button', + { + type: 'button', + 'data-test-button-value1': 'some-value-for-test', + 'data-test-button-value2': 'some-value-for-test', + }, + 'Test', + ); + + const LazyButton = lazy(async () => ({default: Button})); + + const App = () => + React.createElement( + 'html', + null, + React.createElement('head', null), + React.createElement( + 'body', + null, + React.createElement( + Suspense, + {fallback: React.createElement('h1', null, 'Loading...')}, + React.createElement(LazyButton), + ), + ), + ); + + const {writable, output, completed} = getTestWritable(); + + let allReadyCalled = false; + + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + React.createElement(App), + { + onAllReady() { + allReadyCalled = true; + pipe(writable); + }, + }, + ); + }); + + await completed; + + expect(allReadyCalled).toBe(true); + + expect(output.result).not.toContain('$RC'); + expect(output.result).not.toContain('$RV'); + expect(output.result).not.toContain('$RB'); + expect(output.result).not.toContain('$RT'); + + expect(output.result).not.toContain('
'); + + expect(output.result).not.toContain('Loading...'); + }); + + it('should inject streaming scripts in onShellReady with lazy components', async () => { + const Button = () => + React.createElement('button', {type: 'button'}, 'Test'); + + const LazyButton = lazy(async () => ({default: Button})); + + const App = () => + React.createElement( + 'html', + null, + React.createElement('head', null), + React.createElement( + 'body', + null, + React.createElement( + Suspense, + {fallback: React.createElement('h1', null, 'Loading...')}, + React.createElement(LazyButton), + ), + ), + ); + + const {writable, output, completed} = getTestWritable(); + + let shellReadyCalled = false; + + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + React.createElement(App), + { + onShellReady() { + shellReadyCalled = true; + pipe(writable); + }, + }, + ); + }); + + await completed; + + expect(shellReadyCalled).toBe(true); + + }); +}); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 07408d64e8817..6689241a1b848 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -5757,6 +5757,9 @@ function flushSegment( // because it doesn't make sense to outline something if its parent is going to be // blocked on something later in the stream anyway. !flushingPartialBoundaries && + // We don't outline when all pending tasks are complete (e.g., when using onAllReady) + // because there's no need for streaming instructions if everything is already done. + request.allPendingTasks > 0 && isEligibleForOutlining(request, boundary) && (flushedByteSize + boundary.byteSize > request.progressiveChunkSize || hasSuspenseyContent(boundary.contentState)) @@ -6043,6 +6046,11 @@ function flushCompletedQueues( logRecoverableError(request, error, errorInfo, null); } } + // If all pending tasks are complete, we don't need blocking render instructions + // because everything is already done (e.g., when using onAllReady for SEO crawlers). + if (request.allPendingTasks === 0) { + skipBlockingShell = true; + } flushPreamble( request, destination,