Skip to content

Commit 92e3384

Browse files
committed
Merge pull request facebook#1513 from danielschonfeld/shouldcomponentupdate-boolean
Warn if shouldComponentUpdate returns anything other than true/false
2 parents 29083d0 + 7d91277 commit 92e3384

File tree

2 files changed

+48
-3
lines changed

2 files changed

+48
-3
lines changed

src/core/ReactCompositeComponent.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,9 +1005,22 @@ var ReactCompositeComponentMixin = {
10051005
this._pendingState = null;
10061006

10071007
try {
1008-
if (this._pendingForceUpdate ||
1009-
!this.shouldComponentUpdate ||
1010-
this.shouldComponentUpdate(nextProps, nextState, nextContext)) {
1008+
var shouldUpdate =
1009+
this._pendingForceUpdate ||
1010+
!this.shouldComponentUpdate ||
1011+
this.shouldComponentUpdate(nextProps, nextState, nextContext);
1012+
1013+
if (__DEV__) {
1014+
if (typeof shouldUpdate === "undefined") {
1015+
console.warn(
1016+
(this.constructor.displayName || 'ReactCompositeComponent') +
1017+
'.shouldComponentUpdate(): Returned undefined instead of a ' +
1018+
'boolean value. Make sure to return true or false.'
1019+
);
1020+
}
1021+
}
1022+
1023+
if (shouldUpdate) {
10111024
this._pendingForceUpdate = false;
10121025
// Will set `this.props`, `this.state` and `this.context`.
10131026
this._performComponentUpdate(

src/core/__tests__/ReactCompositeComponent-test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,38 @@ describe('ReactCompositeComponent', function() {
894894
expect(React.isValidClass(TrickFnComponent)).toBe(false);
895895
});
896896

897+
it('should warn when shouldComponentUpdate() returns undefined', function() {
898+
var warn = console.warn;
899+
console.warn = mocks.getMockFunction();
900+
901+
try {
902+
var Component = React.createClass({
903+
getInitialState: function () {
904+
return {bogus: false};
905+
},
906+
907+
shouldComponentUpdate: function() {
908+
return undefined;
909+
},
910+
911+
render: function() {
912+
return <div />;
913+
}
914+
});
915+
916+
var instance = ReactTestUtils.renderIntoDocument(<Component />);
917+
instance.setState({bogus: true});
918+
919+
expect(console.warn.mock.calls.length).toBe(1);
920+
expect(console.warn.mock.calls[0][0]).toBe(
921+
'Component.shouldComponentUpdate(): Returned undefined instead of a ' +
922+
'boolean value. Make sure to return true or false.'
923+
);
924+
} finally {
925+
console.warn = warn;
926+
}
927+
});
928+
897929
it('should warn when mispelling shouldComponentUpdate', function() {
898930
var warn = console.warn;
899931
console.warn = mocks.getMockFunction();

0 commit comments

Comments
 (0)