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

support ReactDOM.render(..., document) without crashing #26129

Merged
merged 2 commits into from
Feb 8, 2023
Merged

support ReactDOM.render(..., document) without crashing #26129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3feb583
support ReactDOM.render(..., document) without crashing
gnoff Feb 8, 2023
3de6076
typo
gnoff Feb 8, 2023
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
17 changes: 17 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ let JSDOM;
let Stream;
let Scheduler;
let React;
let ReactDOM;
let ReactDOMClient;
let ReactDOMFizzServer;
let document;
Expand All @@ -28,6 +29,7 @@ describe('ReactDOM HostSingleton', () => {
JSDOM = require('jsdom').JSDOM;
Scheduler = require('scheduler');
React = require('react');
ReactDOM = require('react-dom');
ReactDOMClient = require('react-dom/client');
ReactDOMFizzServer = require('react-dom/server');
Stream = require('stream');
Expand Down Expand Up @@ -1007,4 +1009,19 @@ describe('ReactDOM HostSingleton', () => {
</html>,
);
});

// https://github.com/facebook/react/issues/26128
it('(#26128) does not throw when rendering at body', async () => {
ReactDOM.render(<div />, document.body);
});

// https://github.com/facebook/react/issues/26128
it('(#26128) does not throw when rendering at <html>', async () => {
ReactDOM.render(<body />, document.documentElement);
});

// https://github.com/facebook/react/issues/26128
it('(#26128) does not throw when rendering at document', async () => {
ReactDOM.render(<html />, document);
});
});
17 changes: 11 additions & 6 deletions packages/react-dom/src/__tests__/ReactMount-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,17 @@ describe('ReactMount', () => {
const iFrame = document.createElement('iframe');
document.body.appendChild(iFrame);

expect(() =>
ReactDOM.render(<div />, iFrame.contentDocument.body),
).toErrorDev(
'Rendering components directly into document.body is discouraged',
{withoutStack: true},
);
if (gate(flags => flags.enableHostSingletons)) {
// HostSingletons make the warning for document.body unecessary
ReactDOM.render(<div />, iFrame.contentDocument.body);
} else {
expect(() =>
ReactDOM.render(<div />, iFrame.contentDocument.body),
).toErrorDev(
'Rendering components directly into document.body is discouraged',
{withoutStack: true},
);
}
});

it('should account for escaping on a checksum mismatch', () => {
Expand Down
18 changes: 9 additions & 9 deletions packages/react-dom/src/__tests__/validateDOMNesting-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ function expectWarnings(tags, warnings = [], withoutStack = 0) {
element = <Tag>{element}</Tag>;
}

expect(() => ReactDOM.render(element, container)).toErrorDev(warnings, {
withoutStack,
});
if (warnings.length) {
expect(() => ReactDOM.render(element, container)).toErrorDev(warnings, {
withoutStack,
});
}
}

describe('validateDOMNesting', () => {
Expand All @@ -39,8 +41,10 @@ describe('validateDOMNesting', () => {
expectWarnings(
['body', 'datalist', 'option'],
[
'render(): Rendering components directly into document.body is discouraged',
],
gate(flags => !flags.enableHostSingletons)
? 'render(): Rendering components directly into document.body is discouraged'
: null,
].filter(Boolean),
1,
);
expectWarnings(['div', 'a', 'object', 'a']);
Expand Down Expand Up @@ -106,13 +110,9 @@ describe('validateDOMNesting', () => {
expectWarnings(
['body', 'body'],
[
'render(): Rendering components directly into document.body is discouraged',
'validateDOMNesting(...): <body> cannot appear as a child of <body>.\n' +
' in body (at **)',
'Warning: You are mounting a new body component when a previous one has not first unmounted. It is an error to render more than one body component at a time and attributes and children of these components will likely fail in unpredictable ways. Please only render a single instance of <body> and if you need to mount a new one, ensure any previous ones have unmounted first.\n' +
' in body (at **)',
],
1,
);
} else {
expectWarnings(
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom/src/client/ReactDOMLegacy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes';
import type {ReactNodeList} from 'shared/ReactTypes';

import {clearContainer} from 'react-dom-bindings/src/client/ReactDOMHostConfig';
import {
getInstanceFromNode,
isContainerMarkedAsRoot,
Expand Down Expand Up @@ -42,6 +43,7 @@ import {LegacyRoot} from 'react-reconciler/src/ReactRootTags';
import getComponentNameFromType from 'shared/getComponentNameFromType';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {has as hasInstance} from 'shared/ReactInstanceMap';
import {enableHostSingletons} from '../../../shared/ReactFeatureFlags';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;

Expand Down Expand Up @@ -79,6 +81,7 @@ if (__DEV__) {
}

if (
!enableHostSingletons &&
container.nodeType === ELEMENT_NODE &&
((container: any): Element).tagName &&
((container: any): Element).tagName.toUpperCase() === 'BODY'
Expand Down Expand Up @@ -152,10 +155,7 @@ function legacyCreateRootFromDOMContainer(
return root;
} else {
// First clear any existing content.
let rootSibling;
while ((rootSibling = container.lastChild)) {
container.removeChild(rootSibling);
}
clearContainer(container);

if (typeof callback === 'function') {
const originalCallback = callback;
Expand Down