diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index ebf46e20be..75e3b3f0c9 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -12,10 +12,6 @@ src/isomorphic/classic/__tests__/ReactContextValidator-test.js src/renderers/dom/__tests__/ReactDOMProduction-test.js * should throw with an error code in production -src/renderers/dom/shared/__tests__/ReactDOM-test.js -* throws in render() if the mount callback is not a function -* throws in render() if the update callback is not a function - src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should clean up input value tracking * should clean up input textarea tracking diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index c749ed37ab..5d77c18147 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -620,6 +620,8 @@ src/renderers/dom/shared/__tests__/ReactDOM-test.js * should overwrite props.children with children argument * should purge the DOM cache when removing nodes * allow React.DOM factories to be called without warnings +* throws in render() if the mount callback is not a function +* throws in render() if the update callback is not a function * preserves focus * calls focus() on autoFocus elements after they have been mounted to the DOM diff --git a/src/renderers/dom/shared/__tests__/ReactDOM-test.js b/src/renderers/dom/shared/__tests__/ReactDOM-test.js index a569abd37f..25dd09ec83 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOM-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOM-test.js @@ -133,15 +133,15 @@ describe('ReactDOM', () => { var myDiv = document.createElement('div'); expect(() => ReactDOM.render(, myDiv, 'no')).toThrowError( - 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'render(...): Expected the last optional `callback` argument ' + 'to be a function. Instead received: string.' ); expect(() => ReactDOM.render(, myDiv, {})).toThrowError( - 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'render(...): Expected the last optional `callback` argument ' + 'to be a function. Instead received: Object.' ); expect(() => ReactDOM.render(, myDiv, new Foo())).toThrowError( - 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'render(...): Expected the last optional `callback` argument ' + 'to be a function. Instead received: Foo (keys: a, b).' ); }); @@ -164,15 +164,15 @@ describe('ReactDOM', () => { ReactDOM.render(, myDiv); expect(() => ReactDOM.render(, myDiv, 'no')).toThrowError( - 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'render(...): Expected the last optional `callback` argument ' + 'to be a function. Instead received: string.' ); expect(() => ReactDOM.render(, myDiv, {})).toThrowError( - 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'render(...): Expected the last optional `callback` argument ' + 'to be a function. Instead received: Object.' ); expect(() => ReactDOM.render(, myDiv, new Foo())).toThrowError( - 'ReactDOM.render(...): Expected the last optional `callback` argument ' + + 'render(...): Expected the last optional `callback` argument ' + 'to be a function. Instead received: Foo (keys: a, b).' ); }); diff --git a/src/renderers/dom/stack/client/ReactMount.js b/src/renderers/dom/stack/client/ReactMount.js index b9c756ab17..4186e06970 100644 --- a/src/renderers/dom/stack/client/ReactMount.js +++ b/src/renderers/dom/stack/client/ReactMount.js @@ -33,6 +33,7 @@ var invariant = require('invariant'); var setInnerHTML = require('setInnerHTML'); var shouldUpdateReactComponent = require('shouldUpdateReactComponent'); var warning = require('warning'); +var validateCallback = require('validateCallback'); var ATTR_NAME = DOMProperty.ID_ATTRIBUTE_NAME; var ROOT_ATTR_NAME = DOMProperty.ROOT_ATTRIBUTE_NAME; @@ -432,7 +433,7 @@ var ReactMount = { }, _renderSubtreeIntoContainer: function(parentComponent, nextElement, container, callback) { - ReactUpdateQueue.validateCallback(callback, 'ReactDOM.render'); + validateCallback(callback, 'ReactDOM.render'); invariant( React.isValidElement(nextElement), 'ReactDOM.render(): Invalid component element.%s', diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 325e355978..99e9a08416 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -33,31 +33,6 @@ var invariant = require('invariant'); const isArray = Array.isArray; -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 validateCallback(callback, callerName) { - if (typeof callback !== 'function') { - invariant( - false, - '%s(...): Expected the last optional `callback` argument to be a ' + - 'function. Instead received: %s.', - callerName, - formatUnexpectedArgument(callback) - ); - } -} - module.exports = function( scheduleUpdate : (fiber : Fiber, priorityLevel : PriorityLevel) => void, getPriorityContext : () => PriorityLevel, @@ -67,27 +42,18 @@ module.exports = function( const updater = { isMounted, enqueueSetState(instance, partialState, callback) { - if (callback) { - validateCallback(callback, 'setState'); - } const fiber = ReactInstanceMap.get(instance); const priorityLevel = getPriorityContext(); addUpdate(fiber, partialState, callback || null, priorityLevel); scheduleUpdate(fiber, priorityLevel); }, enqueueReplaceState(instance, state, callback) { - if (callback) { - validateCallback(callback, 'replaceState'); - } const fiber = ReactInstanceMap.get(instance); const priorityLevel = getPriorityContext(); addReplaceUpdate(fiber, state, callback || null, priorityLevel); scheduleUpdate(fiber, priorityLevel); }, enqueueForceUpdate(instance, callback) { - if (callback) { - validateCallback(callback, 'forceUpdate'); - } const fiber = ReactInstanceMap.get(instance); const priorityLevel = getPriorityContext(); addForceUpdate(fiber, callback || null, priorityLevel); diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index fb1c6f500b..72fdeb4e06 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -25,6 +25,7 @@ const { TaskPriority, } = require('ReactPriorityLevel'); +const validateCallback = require('validateCallback'); const warning = require('warning'); type PartialState = @@ -204,12 +205,14 @@ function findInsertionPosition(queue, update) : Update | null { // we shouldn't make a copy. // // If the update is cloned, it returns the cloned update. -function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : Update | null { +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; // Warn if an update is scheduled from inside an updater function. - if (__DEV__ && typeof methodName === 'string') { + if (__DEV__) { if (queue1.isProcessing || (queue2 && queue2.isProcessing)) { if (methodName === 'setState') { warning( @@ -287,11 +290,7 @@ function addUpdate( isTopLevelUnmount: false, next: null, }; - if (__DEV__) { - insertUpdate(fiber, update, 'setState'); - } else { - insertUpdate(fiber, update); - } + insertUpdate(fiber, update, 'setState'); } exports.addUpdate = addUpdate; @@ -310,12 +309,7 @@ function addReplaceUpdate( isTopLevelUnmount: false, next: null, }; - - if (__DEV__) { - insertUpdate(fiber, update, 'replaceState'); - } else { - insertUpdate(fiber, update); - } + insertUpdate(fiber, update, 'replaceState'); } exports.addReplaceUpdate = addReplaceUpdate; @@ -333,11 +327,7 @@ function addForceUpdate( isTopLevelUnmount: false, next: null, }; - if (__DEV__) { - insertUpdate(fiber, update, 'forceUpdate'); - } else { - insertUpdate(fiber, update); - } + insertUpdate(fiber, update, 'forceUpdate'); } exports.addForceUpdate = addForceUpdate; @@ -366,7 +356,7 @@ function addTopLevelUpdate( isTopLevelUnmount, next: null, }; - const update2 = insertUpdate(fiber, update); + const update2 = insertUpdate(fiber, update, 'render'); if (isTopLevelUnmount) { // Drop all updates that are lower-priority, so that the tree is not diff --git a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js index dc9b881a5e..72d3e65073 100644 --- a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js +++ b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js @@ -16,26 +16,13 @@ var ReactInstanceMap = require('ReactInstanceMap'); var ReactInstrumentation = require('ReactInstrumentation'); var ReactUpdates = require('ReactUpdates'); -var invariant = require('invariant'); var warning = require('warning'); +var validateCallback = require('validateCallback'); 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) { @@ -147,7 +134,7 @@ var ReactUpdateQueue = { } if (callback) { - ReactUpdateQueue.validateCallback(callback, callerName); + validateCallback(callback, callerName); if (internalInstance._pendingCallbacks) { internalInstance._pendingCallbacks.push(callback); } else { @@ -187,7 +174,7 @@ var ReactUpdateQueue = { internalInstance._pendingReplaceState = true; if (callback) { - ReactUpdateQueue.validateCallback(callback, callerName); + validateCallback(callback, callerName); if (internalInstance._pendingCallbacks) { internalInstance._pendingCallbacks.push(callback); } else { @@ -235,7 +222,7 @@ var ReactUpdateQueue = { queue.push(partialState); if (callback) { - ReactUpdateQueue.validateCallback(callback, callerName); + validateCallback(callback, callerName); if (internalInstance._pendingCallbacks) { internalInstance._pendingCallbacks.push(callback); } else { @@ -253,16 +240,6 @@ 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/utils/validateCallback.js b/src/renderers/shared/utils/validateCallback.js new file mode 100644 index 0000000000..64da06082c --- /dev/null +++ b/src/renderers/shared/utils/validateCallback.js @@ -0,0 +1,40 @@ +/** + * Copyright 2013-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 validateCallback + * @flow + */ + +'use strict'; + +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) { + invariant( + !callback || typeof callback === 'function', + '%s(...): Expected the last optional `callback` argument to be a ' + + 'function. Instead received: %s.', + callerName, + formatUnexpectedArgument(callback) + ); +} + +module.exports = validateCallback;