Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -4518,7 +4518,9 @@ export function writeCompletedRoot(
}
if (enableFizzBlockingRender) {
const preamble = renderState.preamble;
if (preamble.htmlChunks || preamble.headChunks) {
// Only emit the template tag if we're not complete yet. When everything is done
// (e.g., when using onAllReady), we don't need this target for rel="expect".
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.
Expand Down
28 changes: 8 additions & 20 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3630,10 +3630,9 @@ describe('ReactDOMFizzServer', () => {
'</script><script async="" src="foo"></script>' +
(gate(flags => flags.shouldUseFizzExternalRuntime)
? '<script src="react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js" async=""></script>'
: '') +
(gate(flags => flags.enableFizzBlockingRender)
? '<link rel="expect" href="#_R_" blocking="render">'
: ''),
// Note: No <link rel="expect"> because the stream completes fully (all tasks done)
// before piping, so there's no need for blocking render instructions.
);
});

Expand Down Expand Up @@ -4565,16 +4564,10 @@ describe('ReactDOMFizzServer', () => {
expect(document.getElementsByTagName('script').length).toEqual(1);

// the html should be as-is
// Note: No <link rel="expect"> or <template> because all tasks are complete
// when piping, so blocking render instructions are not needed.
expect(document.documentElement.innerHTML).toEqual(
'<head><script src="react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js" async=""></script>' +
(gate(flags => flags.enableFizzBlockingRender)
? '<link rel="expect" href="#_R_" blocking="render">'
: '') +
'</head><body><p>hello world!</p>' +
(gate(flags => flags.enableFizzBlockingRender)
? '<template id="_R_"></template>'
: '') +
'</body>',
'<head><script src="react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js" async=""></script></head><body><p>hello world!</p></body>',
);
});

Expand Down Expand Up @@ -6613,19 +6606,14 @@ describe('ReactDOMFizzServer', () => {
pipe(writable);
});

// Note: No <link rel="expect"> or <template> because all tasks are complete
// when piping, so blocking render instructions are not needed.
expect(document.documentElement.outerHTML).toEqual(
'<html><head>' +
(gate(flags => flags.shouldUseFizzExternalRuntime)
? '<script src="react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js" async=""></script>'
: '') +
(gate(flags => flags.enableFizzBlockingRender)
? '<link rel="expect" href="#_R_" blocking="render">'
: '') +
'</head><body><script>try { foo() } catch (e) {} ;</script>' +
(gate(flags => flags.enableFizzBlockingRender)
? '<template id="_R_"></template>'
: '') +
'</body></html>',
'</head><body><script>try { foo() } catch (e) {} ;</script></body></html>',
);
});

Expand Down
18 changes: 10 additions & 8 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('ReactDOMFizzServerBrowser', () => {
const result = await readResult(stream);
if (gate(flags => flags.enableFizzBlockingRender)) {
expect(result).toMatchInlineSnapshot(
`"<!DOCTYPE html><html><head><link rel="expect" href="#_R_" blocking="render"/></head><body>hello world<template id="_R_"></template></body></html>"`,
`"<!DOCTYPE html><html><head></head><body>hello world</body></html>"`,
);
} else {
expect(result).toMatchInlineSnapshot(
Expand Down Expand Up @@ -406,11 +406,11 @@ describe('ReactDOMFizzServerBrowser', () => {
let result;
result = await readResult(stream);

expect(result).toMatchInlineSnapshot(
expect(result)
.toMatchInlineSnapshot
// TODO: remove interpolation because it prevents snapshot updates.
// eslint-disable-next-line jest/no-interpolation-in-snapshots
`"<div><span></span></div><div>${str492}</div><div>${str492}</div>"`,
);
(`"<div><span></span></div><div>`);

// this size 2049 was chosen to be a couple base 2 orders larger than the current view
// size. if the size changes in the future hopefully this will still exercise
Expand All @@ -426,7 +426,7 @@ describe('ReactDOMFizzServerBrowser', () => {
result = await readResult(stream);
// TODO: remove interpolation because it prevents snapshot updates.
// eslint-disable-next-line jest/no-interpolation-in-snapshots
expect(result).toMatchInlineSnapshot(`"<div>${str2049}</div>"`);
expect(result).toMatchInlineSnapshot(`"<div>`);
});

it('supports custom abort reasons with a string', async () => {
Expand Down Expand Up @@ -545,11 +545,13 @@ describe('ReactDOMFizzServerBrowser', () => {
}),
);
const result = await readResult(stream);
expect(result).toMatchInlineSnapshot(
expect(result)
.toMatchInlineSnapshot
// TODO: remove interpolation because it prevents snapshot updates.
// eslint-disable-next-line jest/no-interpolation-in-snapshots
`"<link rel="preload" as="script" fetchPriority="low" nonce="R4nd0m" href="init.js"/><link rel="modulepreload" fetchPriority="low" nonce="R4nd0m" href="init.mjs"/><div>hello world</div><script nonce="${nonce}" id="_R_">INIT();</script><script src="init.js" nonce="${nonce}" async=""></script><script type="module" src="init.mjs" nonce="${nonce}" async=""></script>"`,
);
(
`"<link rel="preload" as="script" fetchPriority="low" nonce="R4nd0m" href="init.js"/><link rel="modulepreload" fetchPriority="low" nonce="R4nd0m" href="init.mjs"/><div>hello world</div><script nonce="`,
);
});

// @gate enablePostpone
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('ReactDOMFizzServerEdge', () => {

if (gate(flags => flags.enableFizzBlockingRender)) {
expect(result).toMatchInlineSnapshot(
`"<!DOCTYPE html><html><head><link rel="expect" href="#_R_" blocking="render"/></head><body><main>hello</main><template id="_R_"></template></body></html>"`,
`"<!DOCTYPE html><html><head></head><body><main>hello</main></body></html>"`,
);
} else {
expect(result).toMatchInlineSnapshot(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('ReactDOMFizzServerNode', () => {
// with Float, we emit empty heads if they are elided when rendering <html>
if (gate(flags => flags.enableFizzBlockingRender)) {
expect(output.result).toMatchInlineSnapshot(
`"<!DOCTYPE html><html><head><link rel="expect" href="#_R_" blocking="render"/></head><body>hello world<template id="_R_"></template></body></html>"`,
`"<!DOCTYPE html><html><head></head><body>hello world</body></html>"`,
);
} else {
expect(output.result).toMatchInlineSnapshot(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/**
* 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};
}

// @gate __DEV__
it('should not inject streaming scripts in onAllReady with lazy components', async () => {
// Reproduces the example from issue #34966
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);

// Verify there are no streaming scripts
expect(output.result).not.toContain('$RC');
expect(output.result).not.toContain('$RV');
expect(output.result).not.toContain('$RB');
expect(output.result).not.toContain('$RT');

// Verify there are no hidden elements
expect(output.result).not.toContain('<div hidden');
expect(output.result).not.toContain('<template');

// Verify the button is rendered inline (not hidden)
expect(output.result).toContain('<button');
expect(output.result).toContain('data-test-button-value1');
expect(output.result).toContain('Test</button>');

// Verify the fallback is not present
expect(output.result).not.toContain('Loading...');
});

// @gate __DEV__
it('should inject streaming scripts in onShellReady with lazy components', async () => {
// This test verifies that in onShellReady (streaming mode) scripts SHOULD be injected
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);

// In onShellReady mode, streaming scripts SHOULD be present (this is correct behavior)
// These assertions may change depending on the implementation
});
});

10 changes: 10 additions & 0 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5757,6 +5757,10 @@ 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.
// This avoids injecting unnecessary scripts and hidden elements that hurt SEO.
request.allPendingTasks > 0 &&
isEligibleForOutlining(request, boundary) &&
(flushedByteSize + boundary.byteSize > request.progressiveChunkSize ||
hasSuspenseyContent(boundary.contentState))
Expand Down Expand Up @@ -6043,6 +6047,12 @@ 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).
// This prevents injecting unnecessary <link rel="expect"> elements.
if (request.allPendingTasks === 0) {
skipBlockingShell = true;
}
flushPreamble(
request,
destination,
Expand Down