Skip to content

Commit

Permalink
renderToString is a legacy server API which used a trick to avoid hav…
Browse files Browse the repository at this point in the history
…ing the DOCTYPE included when rendering full documents by setting the root formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this was of little consequence but with Float the Root mode started to be used for things like determining if we could flush hoistable elements yet. In issue #27177 we see that hoisted elements can appear before the <html> tag when using a legacy API `renderToString`.

This change makes the FormatContext responsible for providing the DOCTYPE chunks rather than the insertionMode. Alongside this the legacy mode now uses the ROOT_HTML_MODE insertionMode so that it follows standard logic for hoisting.

The reason I went with this approach is it allows us to avoid any runtime checks for whether we should or should not include a doctype. The only runtime perf consequence is that for legacy APIs when you render <html> there is an extra empty chunk to write which is of tivial consequence
  • Loading branch information
gnoff committed Aug 22, 2023
1 parent b277259 commit b10b923
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 8 deletions.
16 changes: 12 additions & 4 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,11 @@ export function createResponseState(
// Constants for the insertion mode we're currently writing in. We don't encode all HTML5 insertion
// modes. We only include the variants as they matter for the sake of our purposes.
// We don't actually provide the namespace therefore we use constants instead of the string.
const ROOT_HTML_MODE = 0; // Used for the root most element tag.
export const ROOT_HTML_MODE = 0; // Used for the root most element tag.
// We have a less than HTML_HTML_MODE check elsewhere. If you add more cases here, make sure it
// still makes sense
const HTML_HTML_MODE = 1; // Used for the <html> if it is at the top level.
export const HTML_MODE = 2;
const HTML_MODE = 2;
const SVG_MODE = 3;
const MATHML_MODE = 4;
const HTML_TABLE_MODE = 5;
Expand All @@ -392,6 +392,7 @@ export type FormatContext = {
insertionMode: InsertionMode, // root/svg/html/mathml/table
selectedValue: null | string | Array<string>, // the selected value(s) inside a <select>, or null outside <select>
noscriptTagInScope: boolean,
DOCTYPE: PrecomputedChunk,
};

function createFormatContext(
Expand All @@ -403,6 +404,11 @@ function createFormatContext(
insertionMode,
selectedValue,
noscriptTagInScope,
// In practice we only need this for the root format context where it conditions the modern/legacy behavior.
// However to avoid type variance we include it on all contexts. If we ever read this in legacy mode for any
// context other than the root context we would need forward this from parent contexts so the empty doctype
// in legacy mode carries forward. In the current setup this isn't necessary
DOCTYPE,
};
}

Expand Down Expand Up @@ -2676,11 +2682,12 @@ function pushStartHtml(
props: Object,
responseState: ResponseState,
insertionMode: InsertionMode,
doctypeChunk: PrecomputedChunk,
): ReactNodeList {
if (enableFloat) {
if (insertionMode === ROOT_HTML_MODE && responseState.htmlChunks === null) {
// This <html> is the Document.documentElement and should be part of the preamble
responseState.htmlChunks = [DOCTYPE];
responseState.htmlChunks = [doctypeChunk];
return pushStartGenericElement(responseState.htmlChunks, props, 'html');
} else {
// This <html> is deep and is likely just an error. we emit it inline though.
Expand All @@ -2692,7 +2699,7 @@ function pushStartHtml(
// If we're rendering the html tag and we're at the root (i.e. not in foreignObject)
// then we also emit the DOCTYPE as part of the root content as a convenience for
// rendering the whole document.
target.push(DOCTYPE);
target.push(doctypeChunk);
}
return pushStartGenericElement(target, props, 'html');
}
Expand Down Expand Up @@ -3196,6 +3203,7 @@ export function pushStartInstance(
props,
responseState,
formatContext.insertionMode,
formatContext.DOCTYPE,
);
}
default: {
Expand Down
15 changes: 11 additions & 4 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
writeStartClientRenderedSuspenseBoundary as writeStartClientRenderedSuspenseBoundaryImpl,
writeEndCompletedSuspenseBoundary as writeEndCompletedSuspenseBoundaryImpl,
writeEndClientRenderedSuspenseBoundary as writeEndClientRenderedSuspenseBoundaryImpl,
HTML_MODE,
ROOT_HTML_MODE,
} from './ReactFizzConfigDOM';

import type {
Expand Down Expand Up @@ -104,11 +104,20 @@ export function createResponseState(
};
}

import {
stringToChunk,
stringToPrecomputedChunk,
} from 'react-server/src/ReactServerStreamConfig';

// this chunk is empty on purpose because we do not want to emit the DOCTYPE in legacy mode
const DOCTYPE = stringToPrecomputedChunk('');

export function createRootFormatContext(): FormatContext {
return {
insertionMode: HTML_MODE, // We skip the root mode because we don't want to emit the DOCTYPE in legacy mode.
insertionMode: ROOT_HTML_MODE,
selectedValue: null,
noscriptTagInScope: false,
DOCTYPE,
};
}

Expand Down Expand Up @@ -148,8 +157,6 @@ export {
prepareHostDispatcher,
} from './ReactFizzConfigDOM';

import {stringToChunk} from 'react-server/src/ReactServerStreamConfig';

import escapeTextForBrowser from './escapeTextForBrowser';

export function pushTextInstance(
Expand Down
41 changes: 41 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMLegacyFloat-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* 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 ./scripts/jest/ReactDOMServerIntegrationEnvironment
*/

'use strict';

let React;
let ReactDOMFizzServer;

describe('ReactDOMFloat', () => {
beforeEach(() => {
jest.resetModules();

React = require('react');
ReactDOMFizzServer = require('react-dom/server');
});

// fixes #27177
// @gate enableFloat
it('does not hoist above the <html> tag', async () => {
const result = ReactDOMFizzServer.renderToString(
<html>
<head>
<script src="foo" />
<meta charSet="utf-8" />
<title>title</title>
</head>
</html>,
);

expect(result).toEqual(
'<html><head><meta charSet="utf-8"/><title>title</title><script src="foo"></script></head></html>',
);
});
});

0 comments on commit b10b923

Please sign in to comment.