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
Josh Story committed
Feb 8, 2023 at 11:32 UTC
a3152eda5f89e20f056521855f7fa101ce50e4c3
4 files changed
+41
-19
packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js
+17
@@ -13,6 +13,7 @@ let JSDOM;
13
let Stream;
14
let Scheduler;
15
let React;
16
+let ReactDOM;
17
let ReactDOMClient;
18
let ReactDOMFizzServer;
19
let document;
@@ -28,6 +29,7 @@ describe('ReactDOM HostSingleton', () => {
29
JSDOM = require('jsdom').JSDOM;
30
Scheduler = require('scheduler');
31
React = require('react');
32
+ ReactDOM = require('react-dom');
33
ReactDOMClient = require('react-dom/client');
34
ReactDOMFizzServer = require('react-dom/server');
35
Stream = require('stream');
@@ -1007,4 +1009,19 @@ describe('ReactDOM HostSingleton', () => {
1009
</html>,
1010
);
1011
});
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
+ });
1027
});
packages/react-dom/src/__tests__/ReactMount-test.js
+11
-6
@@ -149,12 +149,17 @@ describe('ReactMount', () => {
149
const iFrame = document.createElement('iframe');
150
document.body.appendChild(iFrame);
151
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
+ }
163
});
164
165
it('should account for escaping on a checksum mismatch', () => {
packages/react-dom/src/__tests__/validateDOMNesting-test.js
+9
-9
@@ -28,9 +28,11 @@ function expectWarnings(tags, warnings = [], withoutStack = 0) {
28
element = <Tag>{element}</Tag>;
29
}
30
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
+ }
36
}
37
38
describe('validateDOMNesting', () => {
@@ -39,8 +41,10 @@ describe('validateDOMNesting', () => {
41
expectWarnings(
42
['body', 'datalist', 'option'],
43
[
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),
48
1,
49
);
50
expectWarnings(['div', 'a', 'object', 'a']);
@@ -106,13 +110,9 @@ describe('validateDOMNesting', () => {
110
expectWarnings(
111
['body', 'body'],
112
[
109
- 'render(): Rendering components directly into document.body is discouraged',
113
'validateDOMNesting(...): <body> cannot appear as a child of <body>.\n' +
114
' 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 **)',
115
],
115
- 1,
116
);
117
} else {
118
expectWarnings(
packages/react-dom/src/client/ReactDOMLegacy.js
+4
-4
@@ -14,6 +14,7 @@ import type {
14
import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes';
15
import type {ReactNodeList} from 'shared/ReactTypes';
16
17
+import {clearContainer} from 'react-dom-bindings/src/client/ReactDOMHostConfig';
18
import {
19
getInstanceFromNode,
20
isContainerMarkedAsRoot,
@@ -42,6 +43,7 @@ import {LegacyRoot} from 'react-reconciler/src/ReactRootTags';
43
import getComponentNameFromType from 'shared/getComponentNameFromType';
44
import ReactSharedInternals from 'shared/ReactSharedInternals';
45
import {has as hasInstance} from 'shared/ReactInstanceMap';
46
+import {enableHostSingletons} from '../../../shared/ReactFeatureFlags';
47
48
const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
49
@@ -79,6 +81,7 @@ if (__DEV__) {
81
}
82
83
if (
84
+ !enableHostSingletons &&
85
container.nodeType === ELEMENT_NODE &&
86
((container: any): Element).tagName &&
87
((container: any): Element).tagName.toUpperCase() === 'BODY'
@@ -152,10 +155,7 @@ function legacyCreateRootFromDOMContainer(
155
return root;
156
} else {
157
// First clear any existing content.
155
- let rootSibling;
156
- while ((rootSibling = container.lastChild)) {
157
- container.removeChild(rootSibling);
158
- }
158
+ clearContainer(container);
159
160
if (typeof callback === 'function') {
161
const originalCallback = callback;