Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber] ReactCompositeComponent-test.js #8950

Merged
merged 6 commits into from
Feb 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions scripts/fiber/tests-passing-except-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions src/renderers/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1028,11 +1028,11 @@ describe('ReactCompositeComponent', () => {

ReactTestUtils.renderIntoDocument(<Outer />);
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.'
);
});

Expand Down
4 changes: 4 additions & 0 deletions src/renderers/shared/fiber/ReactDebugCurrentFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

import type { Fiber } from 'ReactFiber';

type LifeCyclePhase = 'render' | 'getChildContext';

if (__DEV__) {
var getComponentName = require('getComponentName');
var { getStackAddendumByWorkInProgressFiber } = require('ReactComponentTreeHook');
Expand Down Expand Up @@ -47,6 +49,8 @@ function getCurrentFiberStackAddendum() : string | null {

var ReactDebugCurrentFiber = {
current: (null : Fiber | null),
phase: (null : LifeCyclePhase | null),

getCurrentFiberOwnerName,
getCurrentFiberStackAddendum,
};
Expand Down
11 changes: 10 additions & 1 deletion src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

if (__DEV__) {
ReactCurrentOwner.current = workInProgress;
ReactDebugCurrentFiber.phase = 'render';
nextChildren = fn(nextProps, context);
ReactDebugCurrentFiber.phase = null;
} else {
nextChildren = fn(nextProps, context);
}
Expand Down Expand Up @@ -279,7 +281,14 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

// 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.
Expand Down
11 changes: 10 additions & 1 deletion src/renderers/shared/fiber/ReactFiberContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const {
if (__DEV__) {
var checkReactTypeSpec = require('checkReactTypeSpec');
var ReactDebugCurrentFrame = require('ReactDebugCurrentFrame');
var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
var warnedAboutMissingGetChildContext = {};
}

Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions src/renderers/shared/fiber/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -144,6 +146,19 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
} = 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;
Expand Down
28 changes: 28 additions & 0 deletions src/renderers/shared/fiber/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,26 @@ if (__DEV__) {
ctor && (ctor.displayName || ctor.name) || 'ReactClass'
);
};

var warnAboutInvalidUpdates = function(instance : ReactClass<any>) {
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;
Expand Down Expand Up @@ -859,6 +879,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
ReactCurrentOwner.current = null;
if (__DEV__) {
ReactDebugCurrentFiber.current = null;
ReactDebugCurrentFiber.phase = null;
}
// It is no longer valid because this unit of work failed.
nextUnitOfWork = null;
Expand Down Expand Up @@ -1102,6 +1123,13 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(config : HostConfig<T, P,
nextUnitOfWork = null;
}

if (__DEV__) {
if (fiber.tag === ClassComponent) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this could be relevant to top level ReactDOM.render too?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you have a separate one for that. Makes sense.

const instance = fiber.stateNode;
warnAboutInvalidUpdates(instance);
}
}

let node = fiber;
let shouldContinue = true;
while (node !== null && shouldContinue) {
Expand Down