diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 050dd41e1b5..d6733db76ea 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -554,6 +554,7 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * 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 +* treats mocked render functions as if they return null src/renderers/dom/shared/__tests__/CSSProperty-test.js * should generate browser prefixes for its `isUnitlessNumber` diff --git a/scripts/jest/test-framework-setup.js b/scripts/jest/test-framework-setup.js index 6128ed6ed80..dc8fd2fc7da 100644 --- a/scripts/jest/test-framework-setup.js +++ b/scripts/jest/test-framework-setup.js @@ -9,6 +9,12 @@ jest.mock('ReactDOMFeatureFlags', () => { useFiber: flags.useFiber || !!process.env.REACT_DOM_JEST_USE_FIBER, }); }); +jest.mock('ReactFeatureFlags', () => { + const flags = require.requireActual('ReactFeatureFlags'); + return Object.assign({}, flags, { + disableNewFiberFeatures: true, + }); +}); // Error logging varies between Fiber and Stack; // Rather than fork dozens of tests, mock the error-logging file by default. diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 8a33133ad4b..7045a874e11 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -18,6 +18,7 @@ import type { ReactNodeList } from 'ReactTypes'; var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); var ReactControlledComponent = require('ReactControlledComponent'); var ReactDOMComponentTree = require('ReactDOMComponentTree'); +var ReactFeatureFlags = require('ReactFeatureFlags'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); var ReactDOMFiberComponent = require('ReactDOMFiberComponent'); var ReactDOMFrameScheduling = require('ReactDOMFrameScheduling'); @@ -27,6 +28,8 @@ var ReactFiberReconciler = require('ReactFiberReconciler'); var ReactInputSelection = require('ReactInputSelection'); var ReactInstanceMap = require('ReactInstanceMap'); var ReactPortal = require('ReactPortal'); +var { isValidElement } = require('ReactElement'); + var findDOMNode = require('findDOMNode'); var invariant = require('invariant'); @@ -49,6 +52,7 @@ if (__DEV__) { var { updatedAncestorInfo } = validateDOMNesting; } + const DOCUMENT_NODE = 9; ReactDOMInjection.inject(); @@ -352,6 +356,17 @@ var ReactDOM = { render(element : ReactElement, container : DOMContainerElement, callback: ?Function) { validateContainer(container); + + if (ReactFeatureFlags.disableNewFiberFeatures) { + // 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.' + ); + } + return renderSubtreeIntoContainer(null, element, container, callback); }, diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js index 41c7f129b79..e5873643bd9 100644 --- a/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js +++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js @@ -18,9 +18,17 @@ var ReactTestUtils = require('ReactTestUtils'); describe('ReactDOMFiber', () => { var container; + var ReactFeatureFlags; beforeEach(() => { container = document.createElement('div'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; + }); + + afterEach(() => { + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = true; }); it('should render strings as children', () => { @@ -1085,40 +1093,49 @@ describe('ReactDOMFiber', () => { container ); }); + } +}); - describe('disableNewFiberFeatures', () => { - var ReactFeatureFlags = require('ReactFeatureFlags'); +// disableNewFiberFeatures currently defaults to true in test +describe('disableNewFiberFeatures', () => { + var container; + var ReactFeatureFlags; - beforeEach(() => { - ReactFeatureFlags.disableNewFiberFeatures = true; - }); + beforeEach(() => { + container = document.createElement('div'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = true; + }); - afterEach(() => { - ReactFeatureFlags.disableNewFiberFeatures = false; - }); + afterEach(() => { + ReactFeatureFlags = require('ReactFeatureFlags'); + 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 non-element passed to top-level render', () => { + 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; - } + 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/); - }); - }); - } + 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/); + }); + + it('treats mocked render functions as if they return null', () => { + class Mocked extends React.Component {} + Mocked.prototype.render = jest.fn(); + ReactDOM.render(, container); + expect(container.textContent).toEqual(''); + }); }); diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index c741d94bddf..95cea880cf9 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -64,7 +64,6 @@ const { FunctionalComponent, ClassComponent, HostText, - HostRoot, HostPortal, CoroutineComponent, YieldComponent, @@ -75,7 +74,6 @@ const { NoEffect, Placement, Deletion, - Err, } = ReactTypeOfSideEffect; function coerceRef(current: ?Fiber, element: ReactElement) { @@ -1104,10 +1102,31 @@ 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) { + const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures; + + // Handle object types + if (typeof newChild === 'object' && newChild !== null) { // 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) { + if (disableNewFiberFeatures) { + 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 + )); + } + } else { switch (newChild.$$typeof) { case REACT_ELEMENT_TYPE: return placeSingleChild(reconcileSingleElement( @@ -1117,6 +1136,22 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { priority )); + case REACT_COROUTINE_TYPE: + return placeSingleChild(reconcileSingleCoroutine( + returnFiber, + currentFirstChild, + newChild, + priority + )); + + case REACT_YIELD_TYPE: + return placeSingleChild(reconcileSingleYield( + returnFiber, + currentFirstChild, + newChild, + priority + )); + case REACT_PORTAL_TYPE: return placeSingleChild(reconcileSinglePortal( returnFiber, @@ -1126,44 +1161,37 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { )); } } + } - 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; - } + if (disableNewFiberFeatures) { + // The new child is not an element. If it's not null or false, + // and the return fiber is a composite component, throw an error. + switch (returnFiber.tag) { + case ClassComponent: { + if (__DEV__) { + const instance = returnFiber.stateNode; + if (instance.render._isMockFunction && typeof newChild === 'undefined') { + // 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' - ); - } + } + // 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(...): 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') { @@ -1175,65 +1203,29 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { )); } - if (typeof newChild === 'object' && newChild !== null) { - switch (newChild.$$typeof) { - case REACT_ELEMENT_TYPE: - return placeSingleChild(reconcileSingleElement( - returnFiber, - currentFirstChild, - newChild, - priority - )); - - case REACT_COROUTINE_TYPE: - return placeSingleChild(reconcileSingleCoroutine( - returnFiber, - currentFirstChild, - newChild, - priority - )); - - case REACT_YIELD_TYPE: - return placeSingleChild(reconcileSingleYield( - returnFiber, - currentFirstChild, - newChild, - priority - )); - - case REACT_PORTAL_TYPE: - return placeSingleChild(reconcileSinglePortal( - returnFiber, - currentFirstChild, - newChild, - priority - )); - } - - if (isArray(newChild)) { - return reconcileChildrenArray( - returnFiber, - currentFirstChild, - newChild, - priority - ); - } + if (isArray(newChild)) { + return reconcileChildrenArray( + returnFiber, + currentFirstChild, + newChild, + priority + ); + } - if (getIteratorFn(newChild)) { - return reconcileChildrenIterator( - returnFiber, - currentFirstChild, - newChild, - priority - ); - } + if (getIteratorFn(newChild)) { + return reconcileChildrenIterator( + returnFiber, + currentFirstChild, + newChild, + priority + ); } - if (typeof newChild === 'undefined') { + 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, + // we already threw above. switch (returnFiber.tag) { - case HostRoot: - // TODO: Top-level render - break; case ClassComponent: { if (__DEV__) { const instance = returnFiber.stateNode; diff --git a/src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js b/src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js index 232582b4078..4ef55100b7f 100644 --- a/src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js @@ -14,6 +14,7 @@ var React; var ReactNoop; var ReactCoroutine; +var ReactFeatureFlags; describe('ReactCoroutine', () => { beforeEach(() => { @@ -21,6 +22,8 @@ describe('ReactCoroutine', () => { React = require('React'); ReactNoop = require('ReactNoop'); ReactCoroutine = require('ReactCoroutine'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); it('should render a coroutine', () => { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 7c3783e7427..d174efd2212 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -13,12 +13,16 @@ var React; var ReactNoop; +var ReactFeatureFlags; describe('ReactIncremental', () => { beforeEach(() => { jest.resetModules(); React = require('React'); ReactNoop = require('ReactNoop'); + + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); it('should render a simple component', () => { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index b3ef77c2793..bef5a4b7b24 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -13,12 +13,15 @@ var React; var ReactNoop; +var ReactFeatureFlags; describe('ReactIncrementalErrorHandling', () => { beforeEach(() => { jest.resetModules(); React = require('React'); ReactNoop = require('ReactNoop'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); function div(...children) { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js index 6d4ff9d1ec8..470d0e278c8 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js @@ -13,12 +13,15 @@ var React; var ReactNoop; +var ReactFeatureFlags; describe('ReactIncrementalReflection', () => { beforeEach(() => { jest.resetModules(); React = require('React'); ReactNoop = require('ReactNoop'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); it('handles isMounted even when the initial render is deferred', () => { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js index 9a12abe4aa1..c856f263805 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js @@ -13,12 +13,15 @@ var React; var ReactNoop; +var ReactFeatureFlags; describe('ReactIncrementalScheduling', () => { beforeEach(() => { jest.resetModules(); React = require('React'); ReactNoop = require('ReactNoop'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); function span(prop) { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js index f8b46fa1866..15b51610e8a 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js @@ -13,12 +13,15 @@ var React; var ReactNoop; +var ReactFeatureFlags; describe('ReactIncrementalSideEffects', () => { beforeEach(() => { jest.resetModules(); React = require('React'); ReactNoop = require('ReactNoop'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); function normalizeCodeLocInfo(str) { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js index 73f97d3db1a..9953c8db81c 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js @@ -13,12 +13,15 @@ var React; var ReactNoop; +var ReactFeatureFlags; describe('ReactIncrementalUpdates', () => { beforeEach(() => { jest.resetModuleRegistry(); React = require('React'); ReactNoop = require('ReactNoop'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); it('applies updates in order of priority', () => { diff --git a/src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js b/src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js index eaff6444762..04004e46a21 100644 --- a/src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js @@ -13,6 +13,7 @@ var React; var ReactNoop; +var ReactFeatureFlags; // This is a new feature in Fiber so I put it in its own test file. It could // probably move to one of the other test files once it is official. @@ -21,6 +22,8 @@ describe('ReactTopLevelFragment', function() { jest.resetModules(); React = require('React'); ReactNoop = require('ReactNoop'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); it('should render a simple fragment at the top of a component', function() { diff --git a/src/renderers/shared/fiber/__tests__/ReactTopLevelText-test.js b/src/renderers/shared/fiber/__tests__/ReactTopLevelText-test.js index 63677123f30..84a98da7301 100644 --- a/src/renderers/shared/fiber/__tests__/ReactTopLevelText-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactTopLevelText-test.js @@ -13,6 +13,7 @@ var React; var ReactNoop; +var ReactFeatureFlags; // This is a new feature in Fiber so I put it in its own test file. It could // probably move to one of the other test files once it is official. @@ -21,6 +22,8 @@ describe('ReactTopLevelText', () => { jest.resetModules(); React = require('React'); ReactNoop = require('ReactNoop'); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; }); it('should render a component returning strings directly from render', () => { diff --git a/src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js b/src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js index 69e74fd211b..ac2c884ec9b 100644 --- a/src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js +++ b/src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js @@ -247,18 +247,24 @@ describe('ReactEmptyComponent', () => { }); it('can render null at the top level', () => { + var ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; var div = document.createElement('div'); - if (ReactDOMFeatureFlags.useFiber) { - ReactDOM.render(null, div); - expect(div.innerHTML).toBe(''); - } else { - // Stack does not implement this. - expect(function() { + try { + if (ReactDOMFeatureFlags.useFiber) { ReactDOM.render(null, div); - }).toThrowError( - 'ReactDOM.render(): Invalid component element.' - ); + expect(div.innerHTML).toBe(''); + } else { + // Stack does not implement this. + expect(function() { + ReactDOM.render(null, div); + }).toThrowError( + 'ReactDOM.render(): Invalid component element.' + ); + } + } finally { + ReactFeatureFlags.disableNewFiberFeatures = true; } }); diff --git a/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js b/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js index b31706c66b5..d02ef0b28cd 100644 --- a/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js +++ b/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js @@ -152,13 +152,8 @@ describe('ReactStatelessComponent', () => { expect(function() { ReactTestUtils.renderIntoDocument(
); }).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.' + '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/testing/__tests__/ReactTestRenderer-test.js b/src/renderers/testing/__tests__/ReactTestRenderer-test.js index 584453d0861..681d48ac022 100644 --- a/src/renderers/testing/__tests__/ReactTestRenderer-test.js +++ b/src/renderers/testing/__tests__/ReactTestRenderer-test.js @@ -14,8 +14,14 @@ var React = require('React'); var ReactTestRenderer = require('ReactTestRenderer'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); +var ReactFeatureFlags; describe('ReactTestRenderer', () => { + beforeEach(() => { + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; + }); + function normalizeCodeLocInfo(str) { return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); }