Merge pull request #8890 from acdlite/fiberfragmentinvariants

[Fiber] Throw on plain object children

scripts/fiber/tests-failing.txt

@@ -1,8 +1,3 @@
-src/addons/__tests__/ReactFragment-test.js
-* should throw if a plain object is used as a child
-* should throw if a plain object even if it is in an owner
-* should throw if a plain object looks like an old element
-
 src/isomorphic/classic/__tests__/ReactContextValidator-test.js
 * should pass previous context to lifecycles
 

scripts/fiber/tests-passing.txt

@@ -39,6 +39,8 @@ src/addons/__tests__/ReactComponentWithPureRenderMixin-test.js
 * does not do a deep comparison
 
 src/addons/__tests__/ReactFragment-test.js
+* should throw if a plain object is used as a child
+* should throw if a plain object even if it is in an owner
 * warns for numeric keys on objects as children
 * should warn if passing null to createFragment
 * should warn if passing an array to createFragment

src/addons/__tests__/ReactFragment-test.js

@@ -59,17 +59,6 @@ describe('ReactFragment', () => {
     );
   });
 
-  it('should throw if a plain object looks like an old element', () => {
-    var oldEl = {_isReactElement: true, type: 'span', props: {}};
-    var container = document.createElement('div');
-    expect(() => ReactDOM.render(<div>{oldEl}</div>, container)).toThrowError(
-      'Objects are not valid as a React child (found: object with keys ' +
-      '{_isReactElement, type, props}). It looks like you\'re using an ' +
-      'element created by a different version of React. Make sure to use ' +
-      'only one copy of React.'
-    );
-  });
-
   it('warns for numeric keys on objects as children', () => {
     spyOn(console, 'error');
 

src/renderers/shared/fiber/ReactChildFiber.js

@@ -36,9 +36,11 @@ var emptyObject = require('emptyObject');
 var getIteratorFn = require('getIteratorFn');
 var invariant = require('invariant');
 var ReactFeatureFlags = require('ReactFeatureFlags');
+var ReactCurrentOwner = require('ReactCurrentOwner');
 
 if (__DEV__) {
   var { getCurrentFiberStackAddendum } = require('ReactDebugCurrentFiber');
+  var { getComponentName } = require('ReactFiberTreeReflection');
   var warning = require('warning');
 }
 
@@ -107,6 +109,34 @@ function coerceRef(current: ?Fiber, element: ReactElement) {
   return mixedRef;
 }
 
+function throwOnInvalidObjectType(returnFiber : Fiber, newChild : Object) {
+  if (returnFiber.type !== 'textarea') {
+    const childrenString = String(newChild);
+    let addendum = '';
+    if (__DEV__) {
+      addendum =
+        ' If you meant to render a collection of children, use an array ' +
+        'instead or wrap the object using createFragment(object) from the ' +
+        'React add-ons.';
+      const owner = ReactCurrentOwner.owner || returnFiber._debugOwner;
+      if (owner && typeof owner.tag === 'number') {
+        const name = getComponentName((owner : any));
+        if (name) {
+          addendum += ' Check the render method of `' + name + '`.';
+        }
+      }
+    }
+    invariant(
+      false,
+      'Objects are not valid as a React child (found: %s).%s',
+      childrenString === '[object Object]' ?
+        'object with keys {' + Object.keys(newChild).join(', ') + '}' :
+        childrenString,
+      addendum
+    );
+  }
+}
+
 // This wrapper function exists because I expect to clone the code in each path
 // to be able to optimize each path individually by branching early. This needs
 // a compiler or we can do it manually. Helpers that don't need this branching
@@ -418,6 +448,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
         created.return = returnFiber;
         return created;
       }
+
+      throwOnInvalidObjectType(returnFiber, newChild);
     }
 
     return null;
@@ -471,6 +503,14 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
             return null;
           }
         }
+
+        case REACT_PORTAL_TYPE: {
+          if (newChild.key === key) {
+            return updatePortal(returnFiber, oldFiber, newChild, priority);
+          } else {
+            return null;
+          }
+        }
       }
 
       if (isArray(newChild) || getIteratorFn(newChild)) {
@@ -481,6 +521,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
         }
         return updateFragment(returnFiber, oldFiber, newChild, priority);
       }
+
+      throwOnInvalidObjectType(returnFiber, newChild);
     }
 
     return null;
@@ -536,6 +578,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
         const matchedFiber = existingChildren.get(newIdx) || null;
         return updateFragment(returnFiber, matchedFiber, newChild, priority);
       }
+
+      throwOnInvalidObjectType(returnFiber, newChild);
     }
 
     return null;
@@ -1083,7 +1127,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
     const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures;
 
     // Handle object types
-    if (typeof newChild === 'object' && newChild !== null) {
+    const isObject = typeof newChild === 'object' && newChild !== null;
+    if (isObject) {
       // Support only the subset of return types that Stack supports. Treat
       // everything else as empty, but log a warning.
       if (disableNewFiberFeatures) {
@@ -1199,6 +1244,10 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
       );
     }
 
+    if (isObject) {
+      throwOnInvalidObjectType(returnFiber, newChild);
+    }
+
     if (!disableNewFiberFeatures && typeof newChild === 'undefined') {
       // If the new child is undefined, and the return fiber is a composite
       // component, throw an error. If Fiber return types are disabled,

src/shared/utils/traverseAllChildren.js

@@ -167,11 +167,6 @@ function traverseAllChildrenImpl(
           ' If you meant to render a collection of children, use an array ' +
           'instead or wrap the object using createFragment(object) from the ' +
           'React add-ons.';
-        if (children._isReactElement) {
-          addendum =
-            ' It looks like you\'re using an element created by a different ' +
-            'version of React. Make sure to use only one copy of React.';
-        }
         if (ReactCurrentOwner.current) {
           var name = ReactCurrentOwner.current.getName();
           if (name) {