Skip to content

Commit

Permalink
Warn when doing createRoot twice on the same node (another approach) (#…
Browse files Browse the repository at this point in the history 
…17329)

* Unify fields used for createRoot warning and event system

* Warn when doing createRoot twice on the same node

* Stricter check for modern roots

* Unmark asynchronously

* Fix Flow
  • Loading branch information
@gaearon
gaearon authored Nov 10, 2019
1 parent be3bfa6 commit a7b4d51
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 31 deletions.
19 changes: 19 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,4 +212,23 @@ describe('ReactDOMRoot', () => {
{withoutStack: true},
);
});

it('warns when creating two roots managing the same container', () => {
ReactDOM.createRoot(container);
expect(() => {
ReactDOM.createRoot(container);
}).toWarnDev(
'You are calling ReactDOM.createRoot() on a container that ' +
'has already been passed to createRoot() before. Instead, call ' +
'root.render() on the existing root instead if you want to update it.',
{withoutStack: true},
);
});

it('does not warn when creating second root after first one is unmounted', () => {
const root = ReactDOM.createRoot(container);
root.unmount();
Scheduler.unstable_flushAll();
ReactDOM.createRoot(container); // No warning
});
});
94 changes: 63 additions & 31 deletions packages/react-dom/src/client/ReactDOM.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ import {
getNodeFromInstance,
getFiberCurrentPropsFromNode,
getClosestInstanceFromNode,
isContainerMarkedAsRoot,
markContainerAsRoot,
unmarkContainerAsRoot,
} from './ReactDOMComponentTree';
import {restoreControlledState} from './ReactDOMComponent';
import {dispatchEvent} from '../events/ReactDOMEventListener';
Expand Down Expand Up @@ -174,11 +176,9 @@ setRestoreImplementation(restoreControlledState);
export type DOMContainer =
| (Element & {
_reactRootContainer: ?_ReactRoot,
_reactHasBeenPassedToCreateRootDEV: ?boolean,
})
| (Document & {
_reactRootContainer: ?_ReactRoot,
_reactHasBeenPassedToCreateRootDEV: ?boolean,
});

type _ReactRoot = {
Expand Down Expand Up @@ -226,22 +226,28 @@ ReactRoot.prototype.render = ReactBlockingRoot.prototype.render = function(
callback: ?() => mixed,
): void {
const root = this._internalRoot;
callback = callback === undefined ? null : callback;
const cb = callback === undefined ? null : callback;
if (__DEV__) {
warnOnInvalidCallback(callback, 'render');
warnOnInvalidCallback(cb, 'render');
}
updateContainer(children, root, null, callback);
updateContainer(children, root, null, cb);
};

ReactRoot.prototype.unmount = ReactBlockingRoot.prototype.unmount = function(
callback: ?() => mixed,
): void {
const root = this._internalRoot;
callback = callback === undefined ? null : callback;
const cb = callback === undefined ? null : callback;
if (__DEV__) {
warnOnInvalidCallback(callback, 'render');
warnOnInvalidCallback(cb, 'render');
}
updateContainer(null, root, null, callback);
const container = root.containerInfo;
updateContainer(null, root, null, () => {
unmarkContainerAsRoot(container);
if (cb !== null) {
cb();
}
});
};

/**
Expand Down Expand Up @@ -448,12 +454,17 @@ const ReactDOM: Object = {
'Target container is not a DOM element.',
);
if (__DEV__) {
warningWithoutStack(
!container._reactHasBeenPassedToCreateRootDEV,
'You are calling ReactDOM.hydrate() on a container that was previously ' +
'passed to ReactDOM.createRoot(). This is not supported. ' +
'Did you mean to call createRoot(container, {hydrate: true}).render(element)?',
);
const isModernRoot =
isContainerMarkedAsRoot(container) &&
container._reactRootContainer === undefined;
if (isModernRoot) {
warningWithoutStack(
false,
'You are calling ReactDOM.hydrate() on a container that was previously ' +
'passed to ReactDOM.createRoot(). This is not supported. ' +
'Did you mean to call createRoot(container, {hydrate: true}).render(element)?',
);
}
}
// TODO: throw or warn if we couldn't hydrate?
return legacyRenderSubtreeIntoContainer(
Expand All @@ -475,12 +486,17 @@ const ReactDOM: Object = {
'Target container is not a DOM element.',
);
if (__DEV__) {
warningWithoutStack(
!container._reactHasBeenPassedToCreateRootDEV,
'You are calling ReactDOM.render() on a container that was previously ' +
'passed to ReactDOM.createRoot(). This is not supported. ' +
'Did you mean to call root.render(element)?',
);
const isModernRoot =
isContainerMarkedAsRoot(container) &&
container._reactRootContainer === undefined;
if (isModernRoot) {
warningWithoutStack(
false,
'You are calling ReactDOM.render() on a container that was previously ' +
'passed to ReactDOM.createRoot(). This is not supported. ' +
'Did you mean to call root.render(element)?',
);
}
}
return legacyRenderSubtreeIntoContainer(
null,
Expand Down Expand Up @@ -521,11 +537,16 @@ const ReactDOM: Object = {
);

if (__DEV__) {
warningWithoutStack(
!container._reactHasBeenPassedToCreateRootDEV,
'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' +
'passed to ReactDOM.createRoot(). This is not supported. Did you mean to call root.unmount()?',
);
const isModernRoot =
isContainerMarkedAsRoot(container) &&
container._reactRootContainer === undefined;
if (isModernRoot) {
warningWithoutStack(
false,
'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' +
'passed to ReactDOM.createRoot(). This is not supported. Did you mean to call root.unmount()?',
);
}
}

if (container._reactRootContainer) {
Expand All @@ -543,6 +564,7 @@ const ReactDOM: Object = {
unbatchedUpdates(() => {
legacyRenderSubtreeIntoContainer(null, null, container, false, () => {
container._reactRootContainer = null;
unmarkContainerAsRoot(container);
});
});
// If you call unmountComponentAtNode twice in quick succession, you'll
Expand Down Expand Up @@ -650,12 +672,22 @@ function createBlockingRoot(

function warnIfReactDOMContainerInDEV(container) {
if (__DEV__) {
warningWithoutStack(
!container._reactRootContainer,
'You are calling ReactDOM.createRoot() on a container that was previously ' +
'passed to ReactDOM.render(). This is not supported.',
);
container._reactHasBeenPassedToCreateRootDEV = true;
if (isContainerMarkedAsRoot(container)) {
if (container._reactRootContainer) {
warningWithoutStack(
false,
'You are calling ReactDOM.createRoot() on a container that was previously ' +
'passed to ReactDOM.render(). This is not supported.',
);
} else {
warningWithoutStack(
false,
'You are calling ReactDOM.createRoot() on a container that ' +
'has already been passed to createRoot() before. Instead, call ' +
'root.render() on the existing root instead if you want to update it.',
);
}
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions packages/react-dom/src/client/ReactDOMComponentTree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ export function markContainerAsRoot(hostRoot, node) {
node[internalContainerInstanceKey] = hostRoot;
}

export function unmarkContainerAsRoot(node) {
node[internalContainerInstanceKey] = null;
}

export function isContainerMarkedAsRoot(node) {
return !!node[internalContainerInstanceKey];
}

// Given a DOM node, return the closest HostComponent or HostText fiber ancestor.
// If the target node is part of a hydrated or not yet rendered subtree, then
// this may also return a SuspenseComponent or HostRoot to indicate that.
Expand Down

0 comments on commit a7b4d51

Please sign in to comment.