diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 36f8480baf3..2316f076e13 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -64,10 +64,6 @@ src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js 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 warn when using non-React functions in JSX - src/renderers/shared/shared/__tests__/ReactUpdates-test.js * marks top-level updates diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 18c4945ca2a..0c7d4082c56 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -544,6 +544,8 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should bubble events from the portal to the parent * should not onMouseLeave when staying in the portal * should not crash encountering low-priority tree +* throws if non-element passed to top-level render +* throws if something other than false, null, or an element is returned from render src/renderers/dom/shared/__tests__/CSSProperty-test.js * should generate browser prefixes for its `isUnitlessNumber` @@ -1536,6 +1538,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 throw on string refs in pure functions * should warn when given a string ref * should warn when given a function ref diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js index 9abec3eedd1..ea7f39da297 100644 --- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js +++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js @@ -1002,5 +1002,40 @@ describe('ReactDOMFiber', () => { container ); }); + + describe('disableNewFiberFeatures', () => { + var ReactFeatureFlags = require('ReactFeatureFlags'); + + beforeEach(() => { + ReactFeatureFlags.disableNewFiberFeatures = true; + }); + + afterEach(() => { + ReactFeatureFlags.disableNewFiberFeatures = false; + }); + + it('throws if non-element passed to top-level render', () => { + // FIXME: These assertions pass individually, but they leave React in + // an inconsistent state. This suggests an error-handling bug. I'll fix + // this in a separate PR. + const message = 'render(): Invalid component element.'; + expect(() => ReactDOM.render(null, container)).toThrow(message, container); + expect(() => ReactDOM.render(undefined, container)).toThrow(message, container); + expect(() => ReactDOM.render(false, container)).toThrow(message, container); + expect(() => ReactDOM.render('Hi', container)).toThrow(message, container); + expect(() => ReactDOM.render(999, container)).toThrow(message, container); + expect(() => ReactDOM.render([
], container)).toThrow(message, container); + }); + + it('throws if something other than false, null, or an element is returned from render', () => { + function Render(props) { + return props.children; + } + + expect(() => ReactDOM.render(Hi, container)).toThrow(/You may have returned undefined/); + expect(() => ReactDOM.render({999}, container)).toThrow(/You may have returned undefined/); + expect(() => ReactDOM.render([
], container)).toThrow(/You may have returned undefined/); + }); + }); } }); diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 9302c2ed809..c741d94bddf 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -36,6 +36,7 @@ var ReactTypeOfWork = require('ReactTypeOfWork'); var emptyObject = require('emptyObject'); var getIteratorFn = require('getIteratorFn'); var invariant = require('invariant'); +var ReactFeatureFlags = require('ReactFeatureFlags'); if (__DEV__) { var { getCurrentFiberStackAddendum } = require('ReactDebugCurrentFiber'); @@ -60,8 +61,10 @@ const { const isArray = Array.isArray; const { + FunctionalComponent, ClassComponent, HostText, + HostRoot, HostPortal, CoroutineComponent, YieldComponent, @@ -72,6 +75,7 @@ const { NoEffect, Placement, Deletion, + Err, } = ReactTypeOfSideEffect; function coerceRef(current: ?Fiber, element: ReactElement) { @@ -1100,6 +1104,68 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { // not as a fragment. Nested arrays on the other hand will be treated as // fragment nodes. Recursion happens at the normal flow. + if (ReactFeatureFlags.disableNewFiberFeatures) { + // Support only the subset of return types that Stack supports. Treat + // everything else as empty, but log a warning. + if (typeof newChild === 'object' && newChild !== null) { + switch (newChild.$$typeof) { + case REACT_ELEMENT_TYPE: + return placeSingleChild(reconcileSingleElement( + returnFiber, + currentFirstChild, + newChild, + priority + )); + + case REACT_PORTAL_TYPE: + return placeSingleChild(reconcileSinglePortal( + returnFiber, + currentFirstChild, + newChild, + priority + )); + } + } + + if (returnFiber.tag === HostRoot) { + // Top-level only accepts elements or portals + invariant( + // If the root has an error effect, this is an intentional unmount. + // Don't throw an error. + returnFiber.effectTag & Err, + 'render(): Invalid component element.' + ); + } else { + switch (returnFiber.tag) { + case ClassComponent: { + if (__DEV__) { + const instance = returnFiber.stateNode; + if (instance.render._isMockFunction) { + // We allow auto-mocks to proceed as if they're + // returning null. + break; + } + } + } + // Intentionally fall through to the next case, which handles both + // functions and classes + // eslint-disable-next-lined no-fallthrough + case FunctionalComponent: { + // Composites accept elements, portals, null, or false + const Component = returnFiber.type; + invariant( + newChild === null || newChild === false, + '%s.render(): 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' + ); + } + } + } + return deleteRemainingChildren(returnFiber, currentFirstChild); + } + if (typeof newChild === 'string' || typeof newChild === 'number') { return placeSingleChild(reconcileSingleTextNode( returnFiber, @@ -1163,6 +1229,36 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { } } + if (typeof newChild === 'undefined') { + switch (returnFiber.tag) { + case HostRoot: + // TODO: Top-level render + break; + case ClassComponent: { + if (__DEV__) { + const instance = returnFiber.stateNode; + if (instance.render._isMockFunction) { + // We allow auto-mocks to proceed as if they're returning null. + break; + } + } + } + // Intentionally fall through to the next case, which handles both + // functions and classes + // eslint-disable-next-lined no-fallthrough + case FunctionalComponent: { + const Component = returnFiber.type; + invariant( + false, + '%s(...): Nothing was returned from render. This usually means a ' + + 'return statement is missing. Or, to render nothing, ' + + 'return null.', + Component.displayName || Component.name || 'Component' + ); + } + } + } + // Remaining cases are all treated as empty. return deleteRemainingChildren(returnFiber, currentFirstChild); } diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index e356056af4c..cb0b5e24720 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -956,6 +956,7 @@ describe('ReactIncremental', () => { function Trail() { ops.push('Trail'); + return null; } function App(props) { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index bee26adef8f..603ab8c7fd2 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/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() { diff --git a/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js b/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js index 060ec461373..876f2685548 100644 --- a/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js +++ b/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
{props.name}
; } @@ -123,18 +125,34 @@ 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 [
,
]; + } + expect(function() { + ReactTestUtils.renderIntoDocument(
); + }).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 [
,
]; } expect(function() { ReactTestUtils.renderIntoDocument(
); - }).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 ? + // Fiber gives a more specific error message for undefined because it + // supports more return types. + 'NotAComponent(...): Nothing was returned from render' : + // Stack's message is generic. + 'NotAComponent(...): A valid React element (or null) must be returned. ' + + 'You may have returned undefined, an array or some other invalid object.' ); }); @@ -374,18 +392,4 @@ describe('ReactStatelessComponent', () => { expect(() => ReactTestUtils.renderIntoDocument()).not.toThrow(); }); - it('should warn when using non-React functions in JSX', () => { - spyOn(console, 'error'); - function NotAComponent() { - return [
,
]; - } - expect(function() { - ReactTestUtils.renderIntoDocument(
); - }).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.' - ); - }); }); diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index 1abe88d6224..ecdccd09824 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -39,26 +39,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); } @@ -205,7 +188,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 || diff --git a/src/renderers/shared/utils/ReactFeatureFlags.js b/src/renderers/shared/utils/ReactFeatureFlags.js index 5bd9651aed1..ce0a442c11b 100644 --- a/src/renderers/shared/utils/ReactFeatureFlags.js +++ b/src/renderers/shared/utils/ReactFeatureFlags.js @@ -18,6 +18,7 @@ var ReactFeatureFlags = { // timeline profiles in Chrome, for example. logTopLevelRenders: false, prepareNewChildrenBeforeUnmountInStack: true, + disableNewFiberFeatures: false, }; module.exports = ReactFeatureFlags;