Merge pull request #8919 from acdlite/fibermounttests

[Fiber] Mount/unmount warnings and invariants

Author: acdlite

scripts/fiber/tests-failing.txt

@@ -15,8 +15,6 @@ src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js
 * can reconcile text arbitrarily split into multiple nodes on some substitutions only
 
 src/renderers/dom/shared/__tests__/ReactMount-test.js
-* throws when given a string
-* throws when given a factory
 * tracks root instances
 * marks top-level mounts
 

scripts/fiber/tests-passing-except-dev.txt

@@ -7,10 +7,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 
 src/renderers/dom/shared/__tests__/ReactMount-test.js
 * should warn if mounting into dirty rendered markup
-* should warn when mounting into document.body
 * should account for escaping on a checksum mismatch
-* should warn if render removes React-rendered children
-* should warn if the unmounted node was rendered by another copy of React
 
 src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js
 * should warn when unmounting a non-container root node

scripts/fiber/tests-passing.txt

@@ -769,10 +769,15 @@ src/renderers/dom/shared/__tests__/ReactEventListener-test.js
 
 src/renderers/dom/shared/__tests__/ReactMount-test.js
 * throws when given a non-node
+* throws when given a string
+* throws when given a factory
 * should render different components in same root
 * should unmount and remount if the key changes
 * should reuse markup if rendering to the same target twice
 * should not warn if mounting into non-empty node
+* should warn when mounting into document.body
+* should warn if render removes React-rendered children
+* should warn if the unmounted node was rendered by another copy of React
 * passes the correct callback context
 
 src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js

src/renderers/dom/fiber/ReactDOMFiber.js

@@ -108,6 +108,18 @@ function validateContainer(container) {
   }
 }
 
+function getReactRootElementInContainer(container : any) {
+  if (!container) {
+    return null;
+  }
+
+  if (container.nodeType === DOC_NODE_TYPE) {
+    return container.documentElement;
+  } else {
+    return container.firstChild;
+  }
+}
+
 function shouldAutoFocusHostComponent(
   type : string,
   props : Props,
@@ -366,9 +378,59 @@ var ReactDOM = {
       // Top-level check occurs here instead of inside child reconciler because
       // because requirements vary between renderers. E.g. React Art
       // allows arrays.
-      invariant(
-        isValidElement(element),
-        'render(): Invalid component element.'
+      if (!isValidElement(element)) {
+        if (typeof element === 'string') {
+          invariant(
+            false,
+            'ReactDOM.render(): Invalid component element. Instead of ' +
+            'passing a string like \'div\', pass ' +
+            'React.createElement(\'div\') or <div />.'
+          );
+        } else if (typeof element === 'function') {
+          invariant(
+            false,
+            'ReactDOM.render(): Invalid component element. Instead of ' +
+            'passing a class like Foo, pass React.createElement(Foo) ' +
+            'or <Foo />.'
+          );
+        } else if (element != null && typeof element.props !== 'undefined') {
+          // Check if it quacks like an element
+          invariant(
+            false,
+            'ReactDOM.render(): Invalid component element. This may be ' +
+            'caused by unintentionally loading two independent copies ' +
+            'of React.'
+          );
+        } else {
+          invariant(
+            false,
+            'ReactDOM.render(): Invalid component element.'
+          );
+        }
+      }
+    }
+
+    if (__DEV__) {
+      const isRootRenderedBySomeReact = Boolean(container._reactRootContainer);
+      const rootEl = getReactRootElementInContainer(container);
+      const hasNonRootReactChild = Boolean(rootEl && ReactDOMComponentTree.getInstanceFromNode(rootEl));
+
+      warning(
+        !hasNonRootReactChild ||
+        isRootRenderedBySomeReact,
+        'render(...): Replacing React-rendered children with a new root ' +
+        'component. If you intended to update the children of this node, ' +
+        'you should instead have the existing children update their state ' +
+        'and render the new components instead of calling ReactDOM.render.'
+      );
+
+      warning(
+        !container.tagName || container.tagName.toUpperCase() !== 'BODY',
+        'render(): Rendering components directly into 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 rendering into a container element created ' +
+        'for your app.'
       );
     }
 
@@ -394,7 +456,18 @@ var ReactDOM = {
       'unmountComponentAtNode(...): Target container is not a DOM element.'
     );
     warnAboutUnstableUse();
+
     if (container._reactRootContainer) {
+      if (__DEV__) {
+        const rootEl = getReactRootElementInContainer(container);
+        const renderedByDifferentReact = rootEl && !ReactDOMComponentTree.getInstanceFromNode(rootEl);
+        warning(
+          !renderedByDifferentReact,
+          'unmountComponentAtNode(): The node you\'re attempting to unmount ' +
+          'was rendered by another copy of React.'
+        );
+      }
+
       // Unmount should not be batched.
       return DOMRenderer.unbatchedUpdates(() => {
         return renderSubtreeIntoContainer(null, null, container, () => {

src/renderers/dom/stack/client/ReactMount.js

@@ -434,23 +434,36 @@ var ReactMount = {
 
   _renderSubtreeIntoContainer: function(parentComponent, nextElement, container, callback) {
     validateCallback(callback, 'ReactDOM.render');
-    invariant(
-      React.isValidElement(nextElement),
-      'ReactDOM.render(): Invalid component element.%s',
-      (
-        typeof nextElement === 'string' ?
-          ' Instead of passing a string like \'div\', pass ' +
-          'React.createElement(\'div\') or <div />.' :
-        typeof nextElement === 'function' ?
-          ' Instead of passing a class like Foo, pass ' +
-          'React.createElement(Foo) or <Foo />.' :
+    if (!React.isValidElement(nextElement)) {
+      if (typeof nextElement === 'string') {
+        invariant(
+          false,
+          'ReactDOM.render(): Invalid component element. Instead of ' +
+          'passing a string like \'div\', pass ' +
+          'React.createElement(\'div\') or <div />.'
+        );
+      } else if (typeof nextElement === 'function') {
+        invariant(
+          false,
+          'ReactDOM.render(): Invalid component element. Instead of ' +
+          'passing a class like Foo, pass React.createElement(Foo) ' +
+          'or <Foo />.'
+        );
+      } else if (nextElement != null && typeof nextElement.props !== 'undefined') {
         // Check if it quacks like an element
-        nextElement != null && nextElement.props !== undefined ?
-          ' This may be caused by unintentionally loading two independent ' +
-          'copies of React.' :
-          ''
-      )
-    );
+        invariant(
+          false,
+          'ReactDOM.render(): Invalid component element. This may be ' +
+          'caused by unintentionally loading two independent copies ' +
+          'of React.'
+        );
+      } else {
+        invariant(
+          false,
+          'ReactDOM.render(): Invalid component element.'
+        );
+      }
+    }
 
     warning(
       !container || !container.tagName ||