diff --git a/src/isomorphic/classic/class/ReactClass.js b/src/isomorphic/classic/class/ReactClass.js index 223e58bf631..8a2a6f30dd5 100644 --- a/src/isomorphic/classic/class/ReactClass.js +++ b/src/isomorphic/classic/class/ReactClass.js @@ -710,7 +710,7 @@ var ReactClassMixin = { replaceState: function(newState, callback) { this.updater.enqueueReplaceState(this, newState); if (callback) { - this.updater.enqueueCallback(this, callback); + this.updater.enqueueCallback(this, callback, 'replaceState'); } }, diff --git a/src/isomorphic/modern/class/ReactComponent.js b/src/isomorphic/modern/class/ReactComponent.js index a7d8b4baa92..a775abd67dd 100644 --- a/src/isomorphic/modern/class/ReactComponent.js +++ b/src/isomorphic/modern/class/ReactComponent.js @@ -76,7 +76,7 @@ ReactComponent.prototype.setState = function(partialState, callback) { } this.updater.enqueueSetState(this, partialState); if (callback) { - this.updater.enqueueCallback(this, callback); + this.updater.enqueueCallback(this, callback, 'setState'); } }; @@ -97,7 +97,7 @@ ReactComponent.prototype.setState = function(partialState, callback) { ReactComponent.prototype.forceUpdate = function(callback) { this.updater.enqueueForceUpdate(this); if (callback) { - this.updater.enqueueCallback(this, callback); + this.updater.enqueueCallback(this, callback, 'forceUpdate'); } }; diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index ac6e6841b09..7f6f8cac6ff 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -382,6 +382,7 @@ var ReactMount = { }, _renderSubtreeIntoContainer: function(parentComponent, nextElement, container, callback) { + ReactUpdateQueue.validateCallback(callback, 'ReactDOM.render'); invariant( ReactElement.isValidElement(nextElement), 'ReactDOM.render(): Invalid component element.%s', diff --git a/src/renderers/dom/client/__tests__/ReactDOM-test.js b/src/renderers/dom/client/__tests__/ReactDOM-test.js index b55381f1d49..9c3cef57f90 100644 --- a/src/renderers/dom/client/__tests__/ReactDOM-test.js +++ b/src/renderers/dom/client/__tests__/ReactDOM-test.js @@ -118,4 +118,63 @@ describe('ReactDOM', function() { expect(console.error.argsForCall.length).toBe(0); }); + it('throws in render() if the mount callback is not a function', function() { + function Foo() { + this.a = 1; + this.b = 2; + } + var A = React.createClass({ + getInitialState: function() { + return {}; + }, + render: function() { + return
; + }, + }); + + var myDiv = document.createElement('div'); + expect(() => ReactDOM.render(, myDiv, 'no')).toThrow( + 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: string.' + ); + expect(() => ReactDOM.render(, myDiv, {})).toThrow( + 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Object.' + ); + expect(() => ReactDOM.render(, myDiv, new Foo())).toThrow( + 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Foo (keys: a, b).' + ); + }); + + it('throws in render() if the update callback is not a function', function() { + function Foo() { + this.a = 1; + this.b = 2; + } + var A = React.createClass({ + getInitialState: function() { + return {}; + }, + render: function() { + return
; + }, + }); + + var myDiv = document.createElement('div'); + ReactDOM.render(, myDiv); + + expect(() => ReactDOM.render(, myDiv, 'no')).toThrow( + 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: string.' + ); + expect(() => ReactDOM.render(, myDiv, {})).toThrow( + 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Object.' + ); + expect(() => ReactDOM.render(, myDiv, new Foo())).toThrow( + 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Foo (keys: a, b).' + ); + }); }); diff --git a/src/renderers/shared/reconciler/ReactUpdateQueue.js b/src/renderers/shared/reconciler/ReactUpdateQueue.js index d2e7cadb28c..65bdd490525 100644 --- a/src/renderers/shared/reconciler/ReactUpdateQueue.js +++ b/src/renderers/shared/reconciler/ReactUpdateQueue.js @@ -22,6 +22,19 @@ function enqueueUpdate(internalInstance) { ReactUpdates.enqueueUpdate(internalInstance); } +function formatUnexpectedArgument(arg) { + var type = typeof arg; + if (type !== 'object') { + return type; + } + var displayName = arg.constructor && arg.constructor.name || type; + var keys = Object.keys(arg); + if (keys.length > 0 && keys.length < 20) { + return `${displayName} (keys: ${keys.join(', ')})`; + } + return displayName; +} + function getInternalInstanceReadyForUpdate(publicInstance, callerName) { var internalInstance = ReactInstanceMap.get(publicInstance); if (!internalInstance) { @@ -103,17 +116,11 @@ var ReactUpdateQueue = { * * @param {ReactClass} publicInstance The instance to use as `this` context. * @param {?function} callback Called after state is updated. + * @param {string} callerName Name of the calling function in the public API. * @internal */ - enqueueCallback: function(publicInstance, callback) { - invariant( - typeof callback === 'function', - 'enqueueCallback(...): You called `setProps`, `replaceProps`, ' + - '`setState`, `replaceState`, or `forceUpdate` with a callback of type ' + - '%s. A function is expected', - typeof callback === 'object' && Object.keys(callback).length && Object.keys(callback).length < 20 ? - typeof callback + ' (keys: ' + Object.keys(callback) + ')' : typeof callback - ); + enqueueCallback: function(publicInstance, callback, callerName) { + ReactUpdateQueue.validateCallback(callback, callerName); var internalInstance = getInternalInstanceReadyForUpdate(publicInstance); // Previously we would throw an error if we didn't have an internal @@ -138,14 +145,6 @@ var ReactUpdateQueue = { }, enqueueCallbackInternal: function(internalInstance, callback) { - invariant( - typeof callback === 'function', - 'enqueueCallback(...): You called `setProps`, `replaceProps`, ' + - '`setState`, `replaceState`, or `forceUpdate` with a callback of type ' + - '%s. A function is expected', - typeof callback === 'object' && Object.keys(callback).length && Object.keys(callback).length < 20 ? - typeof callback + ' (keys: ' + Object.keys(callback) + ')' : typeof callback - ); if (internalInstance._pendingCallbacks) { internalInstance._pendingCallbacks.push(callback); } else { @@ -242,6 +241,16 @@ var ReactUpdateQueue = { enqueueUpdate(internalInstance); }, + validateCallback: function(callback, callerName) { + invariant( + !callback || typeof callback === 'function', + '%s(...): Expected the last optional `callback` argument to be a ' + + 'function. Instead received: %s.', + callerName, + formatUnexpectedArgument(callback) + ); + }, + }; module.exports = ReactUpdateQueue; diff --git a/src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js b/src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js index 22dddac8be5..f59193d691c 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js @@ -937,4 +937,91 @@ describe('ReactUpdates', function() { ReactFeatureFlags.logTopLevelRenders = false; } }); + + it('throws in setState if the update callback is not a function', function() { + function Foo() { + this.a = 1; + this.b = 2; + } + var A = React.createClass({ + getInitialState: function() { + return {}; + }, + render: function() { + return
; + }, + }); + var component = ReactTestUtils.renderIntoDocument(); + + expect(() => component.setState({}, 'no')).toThrow( + 'setState(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: string.' + ); + expect(() => component.setState({}, {})).toThrow( + 'setState(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Object.' + ); + expect(() => component.setState({}, new Foo())).toThrow( + 'setState(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Foo (keys: a, b).' + ); + }); + + it('throws in replaceState if the update callback is not a function', function() { + function Foo() { + this.a = 1; + this.b = 2; + } + var A = React.createClass({ + getInitialState: function() { + return {}; + }, + render: function() { + return
; + }, + }); + var component = ReactTestUtils.renderIntoDocument(); + + expect(() => component.replaceState({}, 'no')).toThrow( + 'replaceState(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: string.' + ); + expect(() => component.replaceState({}, {})).toThrow( + 'replaceState(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Object.' + ); + expect(() => component.replaceState({}, new Foo())).toThrow( + 'replaceState(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Foo (keys: a, b).' + ); + }); + + it('throws in forceUpdate if the update callback is not a function', function() { + function Foo() { + this.a = 1; + this.b = 2; + } + var A = React.createClass({ + getInitialState: function() { + return {}; + }, + render: function() { + return
; + }, + }); + var component = ReactTestUtils.renderIntoDocument(); + + expect(() => component.forceUpdate('no')).toThrow( + 'forceUpdate(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: string.' + ); + expect(() => component.forceUpdate({})).toThrow( + 'forceUpdate(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Object.' + ); + expect(() => component.forceUpdate(new Foo())).toThrow( + 'forceUpdate(...): Expected the last optional `callback` argument ' + + 'to be a function. Instead received: Foo (keys: a, b).' + ); + }); });