Skip to content

Commit a3152ed

Browse files
gnoffgnoff
authored
support ReactDOM.render(..., document) without crashing (#26129)
as reported in #26128 `ReactDOM.render(..., document)` crashed when `enableHostSingletons` was on. This is because it had a different way of clearing the container than `createRoot(document)`. I updated the legacy implementation to share the clearing behavior of `creatRoot` which will preserve the singleton instances. I also removed the warning saying not to use `document.body` as a container
1 parent 758fc7f commit a3152ed

4 files changed

Lines changed: 41 additions & 19 deletions

File tree

‎packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ let JSDOM;
1313
let Stream;
1414
let Scheduler;
1515
let React;
16+
let ReactDOM;
1617
let ReactDOMClient;
1718
let ReactDOMFizzServer;
1819
let document;
@@ -28,6 +29,7 @@ describe('ReactDOM HostSingleton', () => {
2829
JSDOM = require('jsdom').JSDOM;
2930
Scheduler = require('scheduler');
3031
React = require('react');
32+
ReactDOM = require('react-dom');
3133
ReactDOMClient = require('react-dom/client');
3234
ReactDOMFizzServer = require('react-dom/server');
3335
Stream = require('stream');
@@ -1007,4 +1009,19 @@ describe('ReactDOM HostSingleton', () => {
10071009
</html>,
10081010
);
10091011
});
1012+
1013+
// https://github.com/facebook/react/issues/26128
1014+
it('(#26128) does not throw when rendering at body', async () => {
1015+
ReactDOM.render(<div />, document.body);
1016+
});
1017+
1018+
// https://github.com/facebook/react/issues/26128
1019+
it('(#26128) does not throw when rendering at <html>', async () => {
1020+
ReactDOM.render(<body />, document.documentElement);
1021+
});
1022+
1023+
// https://github.com/facebook/react/issues/26128
1024+
it('(#26128) does not throw when rendering at document', async () => {
1025+
ReactDOM.render(<html />, document);
1026+
});
10101027
});

‎packages/react-dom/src/__tests__/ReactMount-test.js‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,17 @@ describe('ReactMount', () => {
149149
const iFrame = document.createElement('iframe');
150150
document.body.appendChild(iFrame);
151151

152-
expect(() =>
153-
ReactDOM.render(<div />, iFrame.contentDocument.body),
154-
).toErrorDev(
155-
'Rendering components directly into document.body is discouraged',
156-
{withoutStack: true},
157-
);
152+
if (gate(flags => flags.enableHostSingletons)) {
153+
// HostSingletons make the warning for document.body unecessary
154+
ReactDOM.render(<div />, iFrame.contentDocument.body);
155+
} else {
156+
expect(() =>
157+
ReactDOM.render(<div />, iFrame.contentDocument.body),
158+
).toErrorDev(
159+
'Rendering components directly into document.body is discouraged',
160+
{withoutStack: true},
161+
);
162+
}
158163
});
159164

160165
it('should account for escaping on a checksum mismatch', () => {

‎packages/react-dom/src/__tests__/validateDOMNesting-test.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ function expectWarnings(tags, warnings = [], withoutStack = 0) {
2828
element = <Tag>{element}</Tag>;
2929
}
3030

31-
expect(() => ReactDOM.render(element, container)).toErrorDev(warnings, {
32-
withoutStack,
33-
});
31+
if (warnings.length) {
32+
expect(() => ReactDOM.render(element, container)).toErrorDev(warnings, {
33+
withoutStack,
34+
});
35+
}
3436
}
3537

3638
describe('validateDOMNesting', () => {
@@ -39,8 +41,10 @@ describe('validateDOMNesting', () => {
3941
expectWarnings(
4042
['body', 'datalist', 'option'],
4143
[
42-
'render(): Rendering components directly into document.body is discouraged',
43-
],
44+
gate(flags => !flags.enableHostSingletons)
45+
? 'render(): Rendering components directly into document.body is discouraged'
46+
: null,
47+
].filter(Boolean),
4448
1,
4549
);
4650
expectWarnings(['div', 'a', 'object', 'a']);
@@ -106,13 +110,9 @@ describe('validateDOMNesting', () => {
106110
expectWarnings(
107111
['body', 'body'],
108112
[
109-
'render(): Rendering components directly into document.body is discouraged',
110113
'validateDOMNesting(...): <body> cannot appear as a child of <body>.\n' +
111114
' in body (at **)',
112-
'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' +
113-
' in body (at **)',
114115
],
115-
1,
116116
);
117117
} else {
118118
expectWarnings(

‎packages/react-dom/src/client/ReactDOMLegacy.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {
1414
import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes';
1515
import type {ReactNodeList} from 'shared/ReactTypes';
1616

17+
import {clearContainer} from 'react-dom-bindings/src/client/ReactDOMHostConfig';
1718
import {
1819
getInstanceFromNode,
1920
isContainerMarkedAsRoot,
@@ -42,6 +43,7 @@ import {LegacyRoot} from 'react-reconciler/src/ReactRootTags';
4243
import getComponentNameFromType from 'shared/getComponentNameFromType';
4344
import ReactSharedInternals from 'shared/ReactSharedInternals';
4445
import {has as hasInstance} from 'shared/ReactInstanceMap';
46+
import {enableHostSingletons} from '../../../shared/ReactFeatureFlags';
4547

4648
const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
4749

@@ -79,6 +81,7 @@ if (__DEV__) {
7981
}
8082

8183
if (
84+
!enableHostSingletons &&
8285
container.nodeType === ELEMENT_NODE &&
8386
((container: any): Element).tagName &&
8487
((container: any): Element).tagName.toUpperCase() === 'BODY'
@@ -152,10 +155,7 @@ function legacyCreateRootFromDOMContainer(
152155
return root;
153156
} else {
154157
// First clear any existing content.
155-
let rootSibling;
156-
while ((rootSibling = container.lastChild)) {
157-
container.removeChild(rootSibling);
158-
}
158+
clearContainer(container);
159159

160160
if (typeof callback === 'function') {
161161
const originalCallback = callback;

0 commit comments

Comments
 (0)
⚡