Skip to content

Commit

Permalink
Ensure createRoot warning parity with ReactDOM.render (#17937)
Browse files Browse the repository at this point in the history
  • Loading branch information
trueadm authored Jan 30, 2020
1 parent b2382a7 commit 1662035
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 1 deletion.
30 changes: 30 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,4 +258,34 @@ describe('ReactDOMRoot', () => {
Scheduler.unstable_flushAll();
ReactDOM.createRoot(container); // No warning
});

it('warns if creating a root on the document.body', async () => {
expect(() => {
ReactDOM.createRoot(document.body);
}).toErrorDev(
'createRoot(): Creating roots directly with document.body is ' +
'discouraged, since its children are often manipulated by third-party ' +
'scripts and browser extensions. This may lead to subtle ' +
'reconciliation issues. Try using a container element created ' +
'for your app.',
{withoutStack: true},
);
});

it('warns if updating a root that has had its contents removed', async () => {
const root = ReactDOM.createRoot(container);
root.render(<div>Hi</div>);
Scheduler.unstable_flushAll();
container.innerHTML = '';

expect(() => {
root.render(<div>Hi</div>);
}).toErrorDev(
'render(...): It looks like the React-rendered content of the ' +
'root container was removed without using React. This is not ' +
'supported and will cause errors. Instead, call ' +
"root.unmount() to empty a root's container.",
{withoutStack: true},
);
});
});
31 changes: 30 additions & 1 deletion packages/react-dom/src/client/ReactDOMRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {ReactNodeList} from 'shared/ReactTypes';
// TODO: This type is shared between the reconciler and ReactDOM, but will
// eventually be lifted out to the renderer.
import type {FiberRoot} from 'react-reconciler/src/ReactFiberRoot';
import {findHostInstanceWithNoPortals} from 'react-reconciler/inline.dom';

export type RootType = {
render(children: ReactNodeList): void,
Expand Down Expand Up @@ -63,15 +64,30 @@ function ReactDOMBlockingRoot(
ReactDOMRoot.prototype.render = ReactDOMBlockingRoot.prototype.render = function(
children: ReactNodeList,
): void {
const root = this._internalRoot;
if (__DEV__) {
if (typeof arguments[1] === 'function') {
console.error(
'render(...): does not support the second callback argument. ' +
'To execute a side effect after rendering, declare it in a component body with useEffect().',
);
}
const container = root.containerInfo;

if (container.nodeType !== COMMENT_NODE) {
const hostInstance = findHostInstanceWithNoPortals(root.current);
if (hostInstance) {
if (hostInstance.parentNode !== container) {
console.error(
'render(...): It looks like the React-rendered content of the ' +
'root container was removed without using React. This is not ' +
'supported and will cause errors. Instead, call ' +
"root.unmount() to empty a root's container.",
);
}
}
}
}
const root = this._internalRoot;
updateContainer(children, root, null, null);
};

Expand Down Expand Up @@ -156,6 +172,19 @@ export function isValidContainer(node: mixed): boolean {

function warnIfReactDOMContainerInDEV(container) {
if (__DEV__) {
if (
container.nodeType === ELEMENT_NODE &&
((container: any): Element).tagName &&
((container: any): Element).tagName.toUpperCase() === 'BODY'
) {
console.error(
'createRoot(): Creating roots directly with document.body is ' +
'discouraged, since its children are often manipulated by third-party ' +
'scripts and browser extensions. This may lead to subtle ' +
'reconciliation issues. Try using a container element created ' +
'for your app.',
);
}
if (isContainerMarkedAsRoot(container)) {
if (container._reactRootContainer) {
console.error(
Expand Down

0 comments on commit 1662035

Please sign in to comment.