diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index e5eb1addeff..db9abfa455f 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -105,11 +105,6 @@ src/renderers/__tests__/ReactComponentTreeHook-test.native.js * reports update counts * does not report top-level wrapper as a root -src/renderers/__tests__/ReactCompositeComponent-test.js -* should warn about `setState` in render -* should warn about `setState` in getChildContext -* should disallow nested render calls - src/renderers/__tests__/ReactHostOperationHistoryHook-test.js * gets recorded for host roots * gets recorded for composite roots diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 525a5586602..ca9ae8de0d6 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -577,6 +577,8 @@ src/renderers/__tests__/ReactCompositeComponent-test.js * should warn about `forceUpdate` on unmounted components * should warn about `setState` on unmounted components * should silently allow `setState`, not call cb on unmounting components +* should warn about `setState` in render +* should warn about `setState` in getChildContext * should cleanup even if render() fatals * should call componentWillUnmount before unmounting * should warn when shouldComponentUpdate() returns undefined @@ -589,6 +591,7 @@ src/renderers/__tests__/ReactCompositeComponent-test.js * should pass context when re-rendered * unmasked context propagates through updates * should trigger componentWillReceiveProps for context changes +* should disallow nested render calls * only renders once if updated in componentWillReceiveProps * only renders once if updated in componentWillReceiveProps when batching * should update refs if shouldComponentUpdate gives false diff --git a/src/renderers/__tests__/ReactCompositeComponent-test.js b/src/renderers/__tests__/ReactCompositeComponent-test.js index ff58a2465cc..03ad27e560b 100644 --- a/src/renderers/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/__tests__/ReactCompositeComponent-test.js @@ -1028,11 +1028,11 @@ describe('ReactCompositeComponent', () => { ReactTestUtils.renderIntoDocument(); expectDev(console.error.calls.count()).toBe(1); - expectDev(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: _renderNewRootComponent(): Render methods should ' + - 'be a pure function of props and state; triggering nested component ' + - 'updates from render is not allowed. If necessary, trigger nested ' + - 'updates in componentDidUpdate.\n\nCheck the render method of Outer.' + expectDev(console.error.calls.argsFor(0)[0]).toMatch( + 'Render methods should be a pure function of props and state; ' + + 'triggering nested component updates from render is not allowed. If ' + + 'necessary, trigger nested updates in componentDidUpdate.\n\nCheck the ' + + 'render method of Outer.' ); }); diff --git a/src/renderers/shared/fiber/ReactDebugCurrentFiber.js b/src/renderers/shared/fiber/ReactDebugCurrentFiber.js index d857e5f2dd3..6969cec6585 100644 --- a/src/renderers/shared/fiber/ReactDebugCurrentFiber.js +++ b/src/renderers/shared/fiber/ReactDebugCurrentFiber.js @@ -14,6 +14,8 @@ import type { Fiber } from 'ReactFiber'; +type LifeCyclePhase = 'render' | 'getChildContext'; + if (__DEV__) { var getComponentName = require('getComponentName'); var { getStackAddendumByWorkInProgressFiber } = require('ReactComponentTreeHook'); @@ -47,6 +49,8 @@ function getCurrentFiberStackAddendum() : string | null { var ReactDebugCurrentFiber = { current: (null : Fiber | null), + phase: (null : LifeCyclePhase | null), + getCurrentFiberOwnerName, getCurrentFiberStackAddendum, }; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 73c4870a300..b88be76bf0e 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -230,7 +230,9 @@ module.exports = function( if (__DEV__) { ReactCurrentOwner.current = workInProgress; + ReactDebugCurrentFiber.phase = 'render'; nextChildren = fn(nextProps, context); + ReactDebugCurrentFiber.phase = null; } else { nextChildren = fn(nextProps, context); } @@ -279,7 +281,14 @@ module.exports = function( // Rerender ReactCurrentOwner.current = workInProgress; - const nextChildren = instance.render(); + let nextChildren; + if (__DEV__) { + ReactDebugCurrentFiber.phase = 'render'; + nextChildren = instance.render(); + ReactDebugCurrentFiber.phase = null; + } else { + nextChildren = instance.render(); + } reconcileChildren(current, workInProgress, nextChildren); // Memoize props and state using the values we just used to render. // TODO: Restructure so we never read values from the instance. diff --git a/src/renderers/shared/fiber/ReactFiberContext.js b/src/renderers/shared/fiber/ReactFiberContext.js index 836bc79eb42..dbb7a43bd99 100644 --- a/src/renderers/shared/fiber/ReactFiberContext.js +++ b/src/renderers/shared/fiber/ReactFiberContext.js @@ -35,6 +35,7 @@ const { if (__DEV__) { var checkReactTypeSpec = require('checkReactTypeSpec'); var ReactDebugCurrentFrame = require('ReactDebugCurrentFrame'); + var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); var warnedAboutMissingGetChildContext = {}; } @@ -168,7 +169,14 @@ function processChildContext(fiber : Fiber, parentContext : Object, isReconcilin return parentContext; } - const childContext = instance.getChildContext(); + let childContext; + if (__DEV__) { + ReactDebugCurrentFiber.phase = 'getChildContext'; + childContext = instance.getChildContext(); + ReactDebugCurrentFiber.phase = null; + } else { + childContext = instance.getChildContext(); + } for (let contextKey in childContext) { invariant( contextKey in childContextTypes, @@ -189,6 +197,7 @@ function processChildContext(fiber : Fiber, parentContext : Object, isReconcilin checkReactTypeSpec(childContextTypes, childContext, 'child context', name); ReactDebugCurrentFrame.current = null; } + return {...parentContext, ...childContext}; } exports.processChildContext = processChildContext; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 00bb5b7d856..3f910aa1d9a 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -32,6 +32,8 @@ var ReactFiberScheduler = require('ReactFiberScheduler'); if (__DEV__) { var warning = require('warning'); var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); + var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); + var { getComponentName } = require('ReactFiberTreeReflection'); } var { findCurrentHostFiber } = require('ReactFiberTreeReflection'); @@ -144,6 +146,19 @@ module.exports = function( } = ReactFiberScheduler(config); function scheduleTopLevelUpdate(current : Fiber, element : ReactNodeList, callback : ?Function) { + if (__DEV__) { + if (ReactDebugCurrentFiber.current !== null) { + warning( + ReactDebugCurrentFiber.phase !== 'render', + 'Render methods should be a pure function of props and state; ' + + 'triggering nested component updates from render is not allowed. ' + + 'If necessary, trigger nested updates in componentDidUpdate.\n\n' + + 'Check the render method of %s.', + getComponentName(ReactDebugCurrentFiber.current) + ); + } + } + const priorityLevel = getPriorityContext(); const nextState = { element }; callback = callback === undefined ? null : callback; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 258a254fab6..f0b8601da34 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -102,6 +102,26 @@ if (__DEV__) { ctor && (ctor.displayName || ctor.name) || 'ReactClass' ); }; + + var warnAboutInvalidUpdates = function(instance : ReactClass) { + switch (ReactDebugCurrentFiber.phase) { + case 'getChildContext': + warning( + false, + 'setState(...): Cannot call setState() inside getChildContext()', + ); + break; + case 'render': + warning( + false, + 'Cannot update during an existing state transition (such as within ' + + '`render` or another component\'s constructor). Render methods should ' + + 'be a pure function of props and state; constructor side-effects are ' + + 'an anti-pattern, but can be moved to `componentWillMount`.' + ); + break; + } + }; } var timeHeuristicForUnitOfWork = 1; @@ -859,6 +879,7 @@ module.exports = function(config : HostConfig(config : HostConfig