[Fiber] Throw on undefined component output

scripts/fiber/tests-failing.txt

@@ -66,16 +66,11 @@ src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js
 src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js
 * should update refs if shouldComponentUpdate gives false
 
-src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js
-* should still throw when rendering to undefined
-
 src/renderers/shared/shared/__tests__/ReactMultiChildText-test.js
 * should reorder keyed text nodes
 
 src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
-* should warn when stateless component returns array
 * should throw on string refs in pure functions
-* should warn when using non-React functions in JSX
 
 src/renderers/shared/shared/__tests__/ReactUpdates-test.js
 * marks top-level updates

scripts/fiber/tests-passing.txt

@@ -1400,6 +1400,7 @@ src/renderers/shared/shared/__tests__/ReactCompositeComponentState-test.js
 
 src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js
 * should not produce child DOM nodes for null and false
+* should still throw when rendering to undefined
 * should be able to switch between rendering null and a normal tag
 * should be able to switch in a list of children
 * should distinguish between a script placeholder and an actual script tag
@@ -1526,6 +1527,7 @@ src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
 * should update stateless component
 * should unmount stateless component
 * should pass context thru stateless component
+* should throw when stateless component returns undefined
 * should provide a null ref
 * should use correct name in key warning
 * should support default props and prop types

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -19,12 +19,19 @@ import type { FiberRoot } from 'ReactFiberRoot';
 import type { HostConfig } from 'ReactFiberReconciler';
 import type { PriorityLevel } from 'ReactPriorityLevel';
 
+var {
+  isValidElement,
+} = require('React');
 var {
   mountChildFibersInPlace,
   reconcileChildFibers,
   reconcileChildFibersInPlace,
   cloneChildFibers,
 } = require('ReactChildFiber');
+var {
+  isCoroutine,
+  isYield,
+} = require('ReactCoroutine');
 var {
   beginUpdateQueue,
 } = require('ReactFiberUpdateQueue');
@@ -49,6 +56,9 @@ var {
   YieldComponent,
   Fragment,
 } = ReactTypeOfWork;
+var {
+  isPortal,
+} = require('ReactPortal');
 var {
   NoWork,
   OffscreenPriority,
@@ -62,6 +72,8 @@ var {
 var ReactCurrentOwner = require('ReactCurrentOwner');
 var ReactFiberClassComponent = require('ReactFiberClassComponent');
 
+var invariant = require('invariant');
+
 if (__DEV__) {
   var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
 }
@@ -88,6 +100,46 @@ module.exports = function<T, P, I, TI, C, CX>(
     updateClassInstance,
   } = ReactFiberClassComponent(scheduleUpdate, getPriorityContext);
 
+  function isValidNode(node) {
+    switch (typeof node) {
+      case 'string':
+      case 'number':
+        return true;
+      case 'boolean':
+        return node === false;
+      case 'object':
+        return (
+          node === null ||
+          Array.isArray(node) ||
+          isValidElement(node) ||
+          isPortal(node) ||
+          isCoroutine(node) ||
+          isYield(node)
+        );
+      case 'undefined':
+        return false;
+      default:
+        return false;
+    }
+  }
+
+  function throwInvalidNodeError() {
+    // TODO: report the node type and/or object keys.
+    var info = '';
+    if (__DEV__) {
+      var name = ReactDebugCurrentFiber.getCurrentFiberOwnerName();
+      if (name) {
+        info += ' Check the render method of `' + name + '`.';
+      }
+    }
+    invariant(
+      false,
+      'A valid React element, null, or an array must be returned. ' +
+      'You may have returned undefined or some other invalid object.%s',
+      info
+    );
+  }
+
   function markChildAsProgressed(current, workInProgress, priorityLevel) {
     // We now have clones. Let's store them as the currently progressed work.
     workInProgress.progressedChild = workInProgress.child;
@@ -211,6 +263,9 @@ module.exports = function<T, P, I, TI, C, CX>(
     } else {
       nextChildren = fn(nextProps, context);
     }
+    if (!isValidNode(nextChildren)) {
+      throwInvalidNodeError();
+    }
     reconcileChildren(current, workInProgress, nextChildren);
     return workInProgress.child;
   }
@@ -263,7 +318,19 @@ module.exports = function<T, P, I, TI, C, CX>(
     // Rerender
     const instance = workInProgress.stateNode;
     ReactCurrentOwner.current = workInProgress;
-    const nextChildren = instance.render();
+    let nextChildren = instance.render();
+    if (__DEV__) {
+      // We allow auto-mocks to proceed as if they're returning null.
+      if (nextChildren === undefined &&
+          instance.render._isMockFunction) {
+        // This is probably bad practice. Consider warning here and
+        // deprecating this convenience.
+        nextChildren = null;
+      }
+    }
+    if (!isValidNode(nextChildren)) {
+      throwInvalidNodeError();
+    }
     reconcileChildren(current, workInProgress, nextChildren);
 
     // The context might have changed so we need to recalculate it.
@@ -433,11 +500,13 @@ module.exports = function<T, P, I, TI, C, CX>(
       adoptClassInstance(workInProgress, value);
       mountClassInstance(workInProgress, priorityLevel);
       return finishClassComponent(current, workInProgress, true, hasContext);
-    } else {
+    } else if (isValidNode(value)) {
       // Proceed under the assumption that this is a functional component
       workInProgress.tag = FunctionalComponent;
       reconcileChildren(current, workInProgress, value);
       return workInProgress.child;
+    } else {
+      throwInvalidNodeError();
     }
   }
 

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

@@ -956,6 +956,7 @@ describe('ReactIncremental', () => {
 
     function Trail() {
       ops.push('Trail');
+      return null;
     }
 
     function App(props) {

src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js

@@ -496,6 +496,7 @@ describe('ReactIncrementalErrorHandling', () => {
       if (props.throw) {
         throw new Error('Hello');
       }
+      return null;
     }
 
     function Foo() {

src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js

@@ -72,15 +72,21 @@ describe('ReactEmptyComponent', () => {
   });
 
   it('should still throw when rendering to undefined', () => {
-    class Component extends React.Component {
+    class MyComponent extends React.Component {
       render() {}
     }
 
     expect(function() {
-      ReactTestUtils.renderIntoDocument(<Component />);
+      ReactTestUtils.renderIntoDocument(<MyComponent />);
     }).toThrowError(
-      'Component.render(): A valid React element (or null) must be returned. You may ' +
-      'have returned undefined, an array or some other invalid object.'
+      ReactDOMFeatureFlags.useFiber ? (
+        'A valid React element, null, or an array must be returned. ' +
+        'You may have returned undefined or some other invalid object. ' +
+        'Check the render method of `MyComponent`.'
+      ) : (
+        'MyComponent.render(): A valid React element (or null) must be returned. You may ' +
+        'have returned undefined, an array or some other invalid object.'
+      )
     );
   });
 

src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js

@@ -15,6 +15,8 @@ var React;
 var ReactDOM;
 var ReactTestUtils;
 
+var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
+
 function StatelessComponent(props) {
   return <div>{props.name}</div>;
 }
@@ -119,18 +121,35 @@ describe('ReactStatelessComponent', () => {
     );
   });
 
-  it('should warn when stateless component returns array', () => {
-    spyOn(console, 'error');
+  if (!ReactDOMFeatureFlags.useFiber) {
+    // Stack doesn't support fragments
+    it('should throw when stateless component returns array', () => {
+      function NotAComponent() {
+        return [<div />, <div />];
+      }
+      expect(function() {
+        ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
+      }).toThrowError(
+        'NotAComponent(...): A valid React element (or null) must be returned. ' +
+        'You may have returned undefined, an array or some other invalid object.'
+      );
+    });
+  }
+
+  it('should throw when stateless component returns undefined', () => {
     function NotAComponent() {
-      return [<div />, <div />];
     }
     expect(function() {
       ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
-    }).toThrow();
-    expectDev(console.error.calls.count()).toBe(1);
-    expectDev(console.error.calls.argsFor(0)[0]).toContain(
-      'NotAComponent(...): A valid React element (or null) must be returned. ' +
-      'You may have returned undefined, an array or some other invalid object.'
+    }).toThrowError(
+      ReactDOMFeatureFlags.useFiber ? (
+        'A valid React element, null, or an array must be returned. ' +
+        'You may have returned undefined or some other invalid object. ' +
+        'Check the render method of `NotAComponent`.'
+      ) : (
+        'NotAComponent(...): A valid React element (or null) must be returned. You may ' +
+        'have returned undefined, an array or some other invalid object.'
+      )
     );
   });
 
@@ -257,18 +276,4 @@ describe('ReactStatelessComponent', () => {
     expect(() => ReactTestUtils.renderIntoDocument(<Child />)).not.toThrow();
   });
 
-  it('should warn when using non-React functions in JSX', () => {
-    spyOn(console, 'error');
-    function NotAComponent() {
-      return [<div />, <div />];
-    }
-    expect(function() {
-      ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
-    }).toThrow();  // has no method 'render'
-    expectDev(console.error.calls.count()).toBe(1);
-    expectDev(console.error.calls.argsFor(0)[0]).toContain(
-      'NotAComponent(...): A valid React element (or null) must be returned. You may ' +
-      'have returned undefined, an array or some other invalid object.'
-    );
-  });
 });

src/renderers/shared/stack/reconciler/ReactCompositeComponent.js

@@ -44,26 +44,9 @@ function StatelessComponent(Component) {
 StatelessComponent.prototype.render = function() {
   var Component = ReactInstanceMap.get(this)._currentElement.type;
   var element = Component(this.props, this.context, this.updater);
-  warnIfInvalidElement(Component, element);
   return element;
 };
 
-function warnIfInvalidElement(Component, element) {
-  if (__DEV__) {
-    warning(
-      element === null || element === false || React.isValidElement(element),
-      '%s(...): A valid React element (or null) must be returned. You may have ' +
-      'returned undefined, an array or some other invalid object.',
-      Component.displayName || Component.name || 'Component'
-    );
-    warning(
-      !Component.childContextTypes,
-      '%s(...): childContextTypes cannot be defined on a functional component.',
-      Component.displayName || Component.name || 'Component'
-    );
-  }
-}
-
 function shouldConstruct(Component) {
   return !!(Component.prototype && Component.prototype.isReactComponent);
 }
@@ -210,7 +193,13 @@ var ReactCompositeComponent = {
     // Support functional components
     if (!doConstruct && (inst == null || inst.render == null)) {
       renderedElement = inst;
-      warnIfInvalidElement(Component, renderedElement);
+      if (__DEV__) {
+        warning(
+          !Component.childContextTypes,
+          '%s(...): childContextTypes cannot be defined on a functional component.',
+          Component.displayName || Component.name || 'Component'
+        );
+      }
       invariant(
         inst === null ||
         inst === false ||