Skip to content

Commit

Permalink
Fix #6114 - Calling setState inside getChildContext should warn
Browse files Browse the repository at this point in the history
  • Loading branch information
rainer oviir authored and rainer oviir committed Mar 5, 2016
1 parent 3b86cb1 commit 9c1916d
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/isomorphic/ReactDebugTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

'use strict';

var ReactInvalidSetStateWarningDevTool = require('ReactInvalidSetStateWarningDevTool');
var warning = require('warning');

var eventHandlers = [];
Expand Down Expand Up @@ -48,6 +49,15 @@ var ReactDebugTool = {
}
}
},
onBeginProcessingChildContext() {
emitEvent('onBeginProcessingChildContext');
},
onEndProcessingChildContext() {
emitEvent('onEndProcessingChildContext');
},
onSetState() {
emitEvent('onSetState');
},
onMountRootComponent(internalInstance) {
emitEvent('onMountRootComponent', internalInstance);
},
Expand All @@ -62,4 +72,6 @@ var ReactDebugTool = {
},
};

ReactDebugTool.addDevtool(ReactInvalidSetStateWarningDevTool);

module.exports = ReactDebugTool;
39 changes: 39 additions & 0 deletions src/isomorphic/devtools/ReactInvalidSetStateWarningDevTool.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* Copyright 2016-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactInvalidSetStateWarningDevTool
*/

'use strict';

var warning = require('warning');

if (__DEV__) {
var processingChildContext = false;

var warnInvalidSetState = function() {
warning(
!processingChildContext,
'setState(...): Cannot call setState() inside getChildContext()'
);
};
}

var ReactInvalidSetStateWarningDevTool = {
onBeginProcessingChildContext() {
processingChildContext = true;
},
onEndProcessingChildContext() {
processingChildContext = false;
},
onSetState() {
warnInvalidSetState();
},
};

module.exports = ReactInvalidSetStateWarningDevTool;
2 changes: 2 additions & 0 deletions src/isomorphic/modern/class/ReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
'use strict';

var ReactNoopUpdateQueue = require('ReactNoopUpdateQueue');
var ReactInstrumentation = require('ReactInstrumentation');

var canDefineProperty = require('canDefineProperty');
var emptyObject = require('emptyObject');
Expand Down Expand Up @@ -66,6 +67,7 @@ ReactComponent.prototype.setState = function(partialState, callback) {
'function which returns an object of state variables.'
);
if (__DEV__) {
ReactInstrumentation.debugTool.onSetState();
warning(
partialState != null,
'setState(...): You passed an undefined or null state object; ' +
Expand Down
7 changes: 7 additions & 0 deletions src/renderers/shared/reconciler/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var ReactCurrentOwner = require('ReactCurrentOwner');
var ReactElement = require('ReactElement');
var ReactErrorUtils = require('ReactErrorUtils');
var ReactInstanceMap = require('ReactInstanceMap');
var ReactInstrumentation = require('ReactInstrumentation');
var ReactNodeTypes = require('ReactNodeTypes');
var ReactPerf = require('ReactPerf');
var ReactPropTypeLocations = require('ReactPropTypeLocations');
Expand Down Expand Up @@ -496,7 +497,13 @@ var ReactCompositeComponentMixin = {
_processChildContext: function(currentContext) {
var Component = this._currentElement.type;
var inst = this._instance;
if (__DEV__) {
ReactInstrumentation.debugTool.onBeginProcessingChildContext();
}
var childContext = inst.getChildContext && inst.getChildContext();
if (__DEV__) {
ReactInstrumentation.debugTool.onEndProcessingChildContext();
}
if (childContext) {
invariant(
typeof Component.childContextTypes === 'object',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,35 @@ describe('ReactCompositeComponent', function() {
expect(instance2.state.value).toBe(1);
});

it('should warn about `setState` in getChildContext', function() {
var container = document.createElement('div');

var renderPasses = 0;

var Component = React.createClass({
getInitialState: function() {
return {value: 0};
},
getChildContext: function() {
if (this.state.value === 0) {
this.setState({ value: 1 });
}
},
render: function() {
renderPasses++;
return <div />;
},
});
expect(console.error.calls.length).toBe(0);
var instance = ReactDOM.render(<Component />, container);
expect(renderPasses).toBe(2);
expect(instance.state.value).toBe(1);
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: setState(...): Cannot call setState() inside getChildContext()'
);
});

it('should cleanup even if render() fatals', function() {
var BadComponent = React.createClass({
render: function() {
Expand Down

0 comments on commit 9c1916d

Please sign in to comment.