Warn when doing createRoot twice on the same node (another approach) (#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

Author: gaearon

packages/react-dom/src/__tests__/ReactDOMRoot-test.js

@@ -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
+  });
 });

packages/react-dom/src/client/ReactDOM.js

@@ -68,7 +68,9 @@ import {
   getNodeFromInstance,
   getFiberCurrentPropsFromNode,
   getClosestInstanceFromNode,
+  isContainerMarkedAsRoot,
   markContainerAsRoot,
+  unmarkContainerAsRoot,
 } from './ReactDOMComponentTree';
 import {restoreControlledState} from './ReactDOMComponent';
 import {dispatchEvent} from '../events/ReactDOMEventListener';
@@ -174,11 +176,9 @@ setRestoreImplementation(restoreControlledState);
 export type DOMContainer =
   | (Element & {
       _reactRootContainer: ?_ReactRoot,
-      _reactHasBeenPassedToCreateRootDEV: ?boolean,
     })
   | (Document & {
       _reactRootContainer: ?_ReactRoot,
-      _reactHasBeenPassedToCreateRootDEV: ?boolean,
     });
 
 type _ReactRoot = {
@@ -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();
+    }
+  });
 };
 
 /**
@@ -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(
@@ -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,
@@ -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) {
@@ -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
@@ -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.',
+        );
+      }
+    }
   }
 }
 

packages/react-dom/src/client/ReactDOMComponentTree.js

@@ -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.