diff --git a/src/renderers/__tests__/ReactUpdates-test.js b/src/renderers/__tests__/ReactUpdates-test.js index 4a455cc0bb..bbe0fc47cb 100644 --- a/src/renderers/__tests__/ReactUpdates-test.js +++ b/src/renderers/__tests__/ReactUpdates-test.js @@ -901,6 +901,8 @@ describe('ReactUpdates', () => { }); it('throws in setState if the update callback is not a function', () => { + spyOn(console, 'error'); + function Foo() { this.a = 1; this.b = 2; @@ -917,20 +919,37 @@ describe('ReactUpdates', () => { var component = ReactTestUtils.renderIntoDocument(); expect(() => component.setState({}, 'no')).toThrowError( - 'setState(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: string.' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - expect(() => component.setState({}, {})).toThrowError( - 'setState(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Object.' + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'setState(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: no.' ); + component = ReactTestUtils.renderIntoDocument(); + expect(() => component.setState({}, {foo: 'bar'})).toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + expectDev(console.error.calls.argsFor(1)[0]).toContain( + 'setState(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + component = ReactTestUtils.renderIntoDocument(); expect(() => component.setState({}, new Foo())).toThrowError( - 'setState(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Foo (keys: a, b).' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); + expectDev(console.error.calls.argsFor(2)[0]).toContain( + 'setState(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + expect(console.error.calls.count()).toBe(3); }); it('throws in replaceState if the update callback is not a function', () => { + spyOn(console, 'error'); + function Foo() { this.a = 1; this.b = 2; @@ -946,20 +965,37 @@ describe('ReactUpdates', () => { var component = ReactTestUtils.renderIntoDocument(); expect(() => component.replaceState({}, 'no')).toThrowError( - 'replaceState(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: string.' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - expect(() => component.replaceState({}, {})).toThrowError( - 'replaceState(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Object.' + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'replaceState(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: no.' ); + component = ReactTestUtils.renderIntoDocument(); + expect(() => component.replaceState({}, {foo: 'bar'})).toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + expectDev(console.error.calls.argsFor(1)[0]).toContain( + 'replaceState(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + component = ReactTestUtils.renderIntoDocument(); expect(() => component.replaceState({}, new Foo())).toThrowError( - 'replaceState(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Foo (keys: a, b).' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); + expectDev(console.error.calls.argsFor(2)[0]).toContain( + 'replaceState(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + expect(console.error.calls.count()).toBe(3); }); it('throws in forceUpdate if the update callback is not a function', () => { + spyOn(console, 'error'); + function Foo() { this.a = 1; this.b = 2; @@ -976,17 +1012,32 @@ describe('ReactUpdates', () => { var component = ReactTestUtils.renderIntoDocument(); expect(() => component.forceUpdate('no')).toThrowError( - 'forceUpdate(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: string.' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - expect(() => component.forceUpdate({})).toThrowError( - 'forceUpdate(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Object.' + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'forceUpdate(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: no.' ); + component = ReactTestUtils.renderIntoDocument(); + expect(() => component.forceUpdate({foo: 'bar'})).toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + expectDev(console.error.calls.argsFor(1)[0]).toContain( + 'forceUpdate(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + component = ReactTestUtils.renderIntoDocument(); expect(() => component.forceUpdate(new Foo())).toThrowError( - 'forceUpdate(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Foo (keys: a, b).' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); + expectDev(console.error.calls.argsFor(2)[0]).toContain( + 'forceUpdate(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + expect(console.error.calls.count()).toBe(3); }); it('does not update one component twice in a batch (#2410)', () => { diff --git a/src/renderers/dom/shared/__tests__/ReactDOM-test.js b/src/renderers/dom/shared/__tests__/ReactDOM-test.js index 25dd09ec83..c7c574a8fb 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOM-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOM-test.js @@ -118,6 +118,8 @@ describe('ReactDOM', () => { }); it('throws in render() if the mount callback is not a function', () => { + spyOn(console, 'error'); + function Foo() { this.a = 1; this.b = 2; @@ -133,20 +135,35 @@ describe('ReactDOM', () => { var myDiv = document.createElement('div'); expect(() => ReactDOM.render(, myDiv, 'no')).toThrowError( - 'render(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: string.' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - expect(() => ReactDOM.render(, myDiv, {})).toThrowError( - 'render(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Object.' + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'render(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: no.' + ); + expect(() => ReactDOM.render(, myDiv, {foo: 'bar'})).toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + expectDev(console.error.calls.argsFor(1)[0]).toContain( + 'render(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' ); expect(() => ReactDOM.render(, myDiv, new Foo())).toThrowError( - 'render(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Foo (keys: a, b).' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); + expectDev(console.error.calls.argsFor(2)[0]).toContain( + 'render(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + expect(console.error.calls.count()).toBe(3); }); it('throws in render() if the update callback is not a function', () => { + spyOn(console, 'error'); + function Foo() { this.a = 1; this.b = 2; @@ -162,19 +179,33 @@ describe('ReactDOM', () => { var myDiv = document.createElement('div'); ReactDOM.render(, myDiv); - expect(() => ReactDOM.render(, myDiv, 'no')).toThrowError( - 'render(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: string.' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - expect(() => ReactDOM.render(, myDiv, {})).toThrowError( - 'render(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Object.' + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'render(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: no.' ); + ReactDOM.render(, myDiv); // Re-mount + expect(() => ReactDOM.render(, myDiv, {foo: 'bar'})).toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', + ); + expectDev(console.error.calls.argsFor(1)[0]).toContain( + 'render(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + ReactDOM.render(, myDiv); // Re-mount expect(() => ReactDOM.render(, myDiv, new Foo())).toThrowError( - 'render(...): Expected the last optional `callback` argument ' + - 'to be a function. Instead received: Foo (keys: a, b).' + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); + expectDev(console.error.calls.argsFor(2)[0]).toContain( + 'render(...): Expected the last optional `callback` argument to be ' + + 'a function. Instead received: [object Object].' + ); + expect(console.error.calls.count()).toBe(3); }); it('preserves focus', () => { diff --git a/src/renderers/dom/stack/client/ReactMount.js b/src/renderers/dom/stack/client/ReactMount.js index cd3fe68503..a1554d81bd 100644 --- a/src/renderers/dom/stack/client/ReactMount.js +++ b/src/renderers/dom/stack/client/ReactMount.js @@ -384,6 +384,7 @@ var ReactMount = { if (callback) { componentInstance._pendingCallbacks = [function() { + validateCallback(callback); callback.call(componentInstance._renderedComponent.getPublicInstance()); }]; } @@ -433,7 +434,15 @@ var ReactMount = { }, _renderSubtreeIntoContainer: function(parentComponent, nextElement, container, callback) { - validateCallback(callback, 'ReactDOM.render'); + callback = callback === undefined ? null : callback; + if (__DEV__) { + warning( + callback === null || typeof callback === 'function', + 'render(...): Expected the last optional `callback` argument to be a ' + + 'function. Instead received: %s.', + String(callback) + ); + } if (!React.isValidElement(nextElement)) { if (typeof nextElement === 'string') { invariant( @@ -489,6 +498,7 @@ var ReactMount = { if (shouldUpdateReactComponent(prevElement, nextElement)) { var publicInst = prevComponent._renderedComponent.getPublicInstance(); var updatedCallback = callback && function() { + validateCallback(callback); callback.call(publicInst); }; ReactMount._updateRootComponent( diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 94fbf67c21..87f3426099 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -35,11 +35,23 @@ var { getComponentName, isMounted } = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); var emptyObject = require('emptyObject'); var shallowEqual = require('shallowEqual'); -var warning = require('warning'); var invariant = require('invariant'); const isArray = Array.isArray; +if (__DEV__) { + var warning = require('warning'); + var warnOnInvalidCallback = function(callback : mixed, callerName : string) { + warning( + callback === null || typeof callback === 'function', + '%s(...): Expected the last optional `callback` argument to be a ' + + 'function. Instead received: %s.', + callerName, + String(callback) + ); + }; +} + module.exports = function( scheduleUpdate : (fiber : Fiber, priorityLevel : PriorityLevel) => void, getPriorityContext : () => PriorityLevel, @@ -53,19 +65,31 @@ module.exports = function( enqueueSetState(instance, partialState, callback) { const fiber = ReactInstanceMap.get(instance); const priorityLevel = getPriorityContext(); - addUpdate(fiber, partialState, callback || null, priorityLevel); + callback = callback === undefined ? null : callback; + if (__DEV__) { + warnOnInvalidCallback(callback, 'setState'); + } + addUpdate(fiber, partialState, callback, priorityLevel); scheduleUpdate(fiber, priorityLevel); }, enqueueReplaceState(instance, state, callback) { const fiber = ReactInstanceMap.get(instance); const priorityLevel = getPriorityContext(); - addReplaceUpdate(fiber, state, callback || null, priorityLevel); + callback = callback === undefined ? null : callback; + if (__DEV__) { + warnOnInvalidCallback(callback, 'replaceState'); + } + addReplaceUpdate(fiber, state, callback, priorityLevel); scheduleUpdate(fiber, priorityLevel); }, enqueueForceUpdate(instance, callback) { const fiber = ReactInstanceMap.get(instance); const priorityLevel = getPriorityContext(); - addForceUpdate(fiber, callback || null, priorityLevel); + callback = callback === undefined ? null : callback; + if (__DEV__) { + warnOnInvalidCallback(callback, 'forceUpdate'); + } + addForceUpdate(fiber, callback, priorityLevel); scheduleUpdate(fiber, priorityLevel); }, }; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 1a429daf0a..d42b582128 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -30,6 +30,7 @@ var { createFiberRoot } = require('ReactFiberRoot'); var ReactFiberScheduler = require('ReactFiberScheduler'); if (__DEV__) { + var warning = require('warning'); var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); } @@ -145,6 +146,15 @@ module.exports = function( function scheduleTopLevelUpdate(current : Fiber, element : ReactNodeList, callback : ?Function) { const priorityLevel = getPriorityContext(); const nextState = { element }; + callback = callback === undefined ? null : callback; + if (__DEV__) { + warning( + callback === null || typeof callback === 'function', + 'render(...): Expected the last optional `callback` argument to be a ' + + 'function. Instead received: %s.', + String(callback) + ); + } addTopLevelUpdate(current, nextState, callback || null, priorityLevel); scheduleUpdate(current, priorityLevel); } diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index bb70bec77d..8bb1d1401d 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -25,14 +25,17 @@ const { TaskPriority, } = require('ReactPriorityLevel'); -const validateCallback = require('validateCallback'); -const warning = require('warning'); +const invariant = require('invariant'); +if (__DEV__) { + var warning = require('warning'); +} type PartialState = $Subtype | (prevState: State, props: Props) => $Subtype; -type Callback = () => void; +// Callbacks are not validated until invocation +type Callback = mixed; type Update = { priorityLevel: PriorityLevel, @@ -215,8 +218,6 @@ function findInsertionPosition(queue, update) : Update | null { // // If the update is cloned, it returns the cloned update. function insertUpdate(fiber : Fiber, update : Update, methodName : string) : Update | null { - validateCallback(update.callback, methodName); - const queue1 = ensureUpdateQueue(fiber); const queue2 = fiber.alternate ? ensureUpdateQueue(fiber.alternate) : null; @@ -287,7 +288,7 @@ function insertUpdate(fiber : Fiber, update : Update, methodName : string) : Upd function addUpdate( fiber : Fiber, partialState : PartialState | null, - callback : Callback | null, + callback : mixed, priorityLevel : PriorityLevel ) : void { const update = { @@ -446,7 +447,7 @@ function beginUpdateQueue( } // Second condition ignores top-level unmount callbacks if they are not the // last update in the queue, since a subsequent update will cause a remount. - if (update.callback && !(update.isTopLevelUnmount && update.next)) { + if (update.callback !== null && !(update.isTopLevelUnmount && update.next !== null)) { callbackList = callbackList || []; callbackList.push(update.callback); workInProgress.effectTag |= CallbackEffect; @@ -476,6 +477,12 @@ function commitCallbacks(finishedWork : Fiber, queue : UpdateQueue, context : mi } for (let i = 0; i < callbackList.length; i++) { const callback = callbackList[i]; + invariant( + typeof callback === 'function', + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: %s', + String(callback) + ); callback.call(context); } } diff --git a/src/renderers/shared/stack/reconciler/CallbackQueue.js b/src/renderers/shared/stack/reconciler/CallbackQueue.js index 84e87cac3a..a6cd976129 100644 --- a/src/renderers/shared/stack/reconciler/CallbackQueue.js +++ b/src/renderers/shared/stack/reconciler/CallbackQueue.js @@ -15,6 +15,7 @@ var PooledClass = require('PooledClass'); var invariant = require('invariant'); +var validateCallback = require('validateCallback'); /** * A specialized pseudo-event module to help keep track of components waiting to @@ -70,6 +71,7 @@ class CallbackQueue { this._callbacks = null; this._contexts = null; for (var i = 0; i < callbacks.length; i++) { + validateCallback(callbacks[i]); callbacks[i].call(contexts[i], arg); } callbacks.length = 0; diff --git a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js index a9bed9832c..3fbdf6c741 100644 --- a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js +++ b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js @@ -16,8 +16,18 @@ var ReactInstanceMap = require('ReactInstanceMap'); var ReactInstrumentation = require('ReactInstrumentation'); var ReactUpdates = require('ReactUpdates'); -var warning = require('warning'); -var validateCallback = require('validateCallback'); +if (__DEV__) { + var warning = require('warning'); + var warnOnInvalidCallback = function(callback : mixed, callerName : string) { + warning( + callback === null || typeof callback === 'function', + '%s(...): Expected the last optional `callback` argument to be a ' + + 'function. Instead received: %s.', + callerName, + String(callback) + ); + }; +} function enqueueUpdate(internalInstance) { ReactUpdates.enqueueUpdate(internalInstance); @@ -134,7 +144,8 @@ var ReactUpdateQueue = { } if (callback) { - validateCallback(callback, callerName); + callback = callback === undefined ? null : callback; + warnOnInvalidCallback(callback, callerName); if (internalInstance._pendingCallbacks) { internalInstance._pendingCallbacks.push(callback); } else { @@ -174,7 +185,8 @@ var ReactUpdateQueue = { internalInstance._pendingReplaceState = true; if (callback) { - validateCallback(callback, callerName); + callback = callback === undefined ? null : callback; + warnOnInvalidCallback(callback, callerName); if (internalInstance._pendingCallbacks) { internalInstance._pendingCallbacks.push(callback); } else { @@ -222,7 +234,8 @@ var ReactUpdateQueue = { queue.push(partialState); if (callback) { - validateCallback(callback, callerName); + callback = callback === undefined ? null : callback; + warnOnInvalidCallback(callback, callerName); if (internalInstance._pendingCallbacks) { internalInstance._pendingCallbacks.push(callback); } else { diff --git a/src/renderers/shared/utils/validateCallback.js b/src/renderers/shared/utils/validateCallback.js index 65c0260db9..657265b84f 100644 --- a/src/renderers/shared/utils/validateCallback.js +++ b/src/renderers/shared/utils/validateCallback.js @@ -14,33 +14,13 @@ const invariant = require('invariant'); -function formatUnexpectedArgument(arg: any) { - let type = typeof arg; - if (type !== 'object') { - return type; - } - let displayName = arg.constructor && arg.constructor.name || type; - let keys = Object.keys(arg); - if (keys.length > 0 && keys.length < 20) { - return `${displayName} (keys: ${keys.join(', ')})`; - } - return displayName; -} - -function validateCallback(callback: ?Function, callerName: string) { - if ( - callback !== null && - callback !== undefined && - typeof callback !== 'function' - ) { - invariant( - false, - '%s(...): Expected the last optional `callback` argument to be a ' + - 'function. Instead received: %s.', - callerName, - formatUnexpectedArgument(callback) - ); - } +function validateCallback(callback: ?Function) { + invariant( + !callback || typeof callback === 'function', + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: %s', + String(callback) + ); } module.exports = validateCallback;