[Fiber]Fix tests for mount/update callback errors

scripts/fiber/tests-failing.txt

@@ -24,10 +24,6 @@ src/renderers/art/__tests__/ReactART-test.js
 * resolves refs before componentDidMount
 * resolves refs before componentDidUpdate
 
-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 empty element when removing innerHTML
 * should transition from innerHTML to children in nested el
@@ -109,9 +105,6 @@ src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
 src/renderers/shared/shared/__tests__/ReactUpdates-test.js
 * should queue mount-ready handlers across different roots
 * marks top-level updates
-* throws in setState if the update callback is not a function
-* throws in replaceState if the update callback is not a function
-* throws in forceUpdate if the update callback is not a function
 
 src/renderers/shared/shared/__tests__/refs-test.js
 * Should increase refs with an increase in divs

scripts/fiber/tests-passing.txt

@@ -577,6 +577,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
 
 src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -1431,6 +1433,9 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js
 * should queue updates from during mount
 * calls componentWillReceiveProps setState callback properly
 * does not call render after a component as been deleted
+* throws in setState if the update callback is not a function
+* throws in replaceState if the update callback is not a function
+* throws in forceUpdate if the update callback is not a function
 * does not update one component twice in a batch (#2410)
 * does not update one component twice in a batch (#6371)
 * unstable_batchedUpdates should return value from a callback

src/renderers/dom/fiber/ReactDOMFiber.js

@@ -165,7 +165,13 @@ function warnAboutUnstableUse() {
   warned = true;
 }
 
-function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any, any>, element : ReactElement<any>, containerNode : DOMContainerElement | Document, callback: ?Function) {
+function renderSubtreeIntoContainer(
+  parentComponent : ?ReactComponent<any, any, any>,
+  element : ReactElement<any>,
+  containerNode : DOMContainerElement | Document,
+  callback: ?Function,
+  callerName: ?string
+) {
   let container : DOMContainerElement =
     containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any);
   let root;
@@ -174,9 +180,10 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any,
     while (container.lastChild) {
       container.removeChild(container.lastChild);
     }
-    root = container._reactRootContainer = DOMRenderer.mountContainer(element, container, parentComponent, callback);
+    root = container._reactRootContainer =
+      DOMRenderer.mountContainer(element, container, parentComponent, callback, callerName);
   } else {
-    DOMRenderer.updateContainer(element, root = container._reactRootContainer, parentComponent, callback);
+    DOMRenderer.updateContainer(element, root = container._reactRootContainer, parentComponent, callback, callerName);
   }
   return DOMRenderer.getPublicRootInstance(root);
 }
@@ -185,15 +192,22 @@ var ReactDOM = {
 
   render(element : ReactElement<any>, container : DOMContainerElement, callback: ?Function) {
     warnAboutUnstableUse();
-    return renderSubtreeIntoContainer(null, element, container, callback);
+    var callerName = 'ReactDOM.render';
+    return renderSubtreeIntoContainer(null, element, container, callback, callerName);
   },
 
-  unstable_renderSubtreeIntoContainer(parentComponent : ReactComponent<any, any, any>, element : ReactElement<any>, containerNode : DOMContainerElement | Document, callback: ?Function) {
+  unstable_renderSubtreeIntoContainer(
+    parentComponent : ReactComponent<any, any, any>,
+    element : ReactElement<any>,
+    containerNode : DOMContainerElement | Document,
+    callback: ?Function
+  ) {
     invariant(
       parentComponent != null && ReactInstanceMap.has(parentComponent),
       'parentComponent must be a valid React Component'
     );
-    return renderSubtreeIntoContainer(parentComponent, element, containerNode, callback);
+    var callerName = 'ReactDOM.unstable_renderSubtreeIntoContainer';
+    return renderSubtreeIntoContainer(parentComponent, element, containerNode, callback, callerName);
   },
 
   unmountComponentAtNode(container : DOMContainerElement) {

src/renderers/noop/ReactNoop.js

@@ -156,19 +156,25 @@ var ReactNoop = {
 
   // Shortcut for testing a single root
   render(element : ReactElement<any>, callback: ?Function) {
-    ReactNoop.renderToRootWithID(element, DEFAULT_ROOT_ID, callback);
+    var callerName = 'ReactNoop.render';
+    ReactNoop.renderToRootWithID(element, DEFAULT_ROOT_ID, callback, callerName);
   },
 
-  renderToRootWithID(element : ReactElement<any>, rootID : string, callback : ?Function) {
+  renderToRootWithID(
+    element : ReactElement<any>,
+    rootID : string,
+    callback : ?Function,
+    callerName: ?string = 'ReactNoop.renderToRootWithID'
+  ) {
     if (!roots.has(rootID)) {
       const container = { rootID: rootID, children: [] };
       rootContainers.set(rootID, container);
-      const root = NoopRenderer.mountContainer(element, container, null, callback);
+      const root = NoopRenderer.mountContainer(element, container, null, callback, callerName);
       roots.set(rootID, root);
     } else {
       const root = roots.get(rootID);
       if (root) {
-        NoopRenderer.updateContainer(element, root, null, callback);
+        NoopRenderer.updateContainer(element, root, null, callback, callerName);
       }
     }
   },

src/renderers/shared/fiber/ReactFiberClassComponent.js

@@ -66,12 +66,12 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
       updateQueue.isForced = true;
       scheduleUpdateQueue(fiber, updateQueue);
     },
-    enqueueCallback(instance, callback) {
+    enqueueCallback(instance, callback, callerName) {
       const fiber = ReactInstanceMap.get(instance);
       let updateQueue = fiber.updateQueue ?
         fiber.updateQueue :
         createUpdateQueue(null);
-      addCallbackToQueue(updateQueue, callback);
+      addCallbackToQueue(updateQueue, callback, callerName);
       scheduleUpdateQueue(fiber, updateQueue);
     },
   };

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -101,13 +101,19 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) :
 
   return {
 
-    mountContainer(element : ReactElement<any>, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : OpaqueNode {
+    mountContainer(
+      element : ReactElement<any>,
+      containerInfo : C,
+      parentComponent : ?ReactComponent<any, any, any>,
+      callback: ?Function,
+      callerName: ?string
+    ) : OpaqueNode {
       const context = getContextForSubtree(parentComponent);
       const root = createFiberRoot(containerInfo, context);
       const container = root.current;
       if (callback) {
         const queue = createUpdateQueue(null);
-        addCallbackToQueue(queue, callback);
+        addCallbackToQueue(queue, callback, ((callerName : any) : string));
         root.callbackList = queue;
       }
       // TODO: Use pending work/state instead of props.
@@ -127,14 +133,20 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) :
       return container;
     },
 
-    updateContainer(element : ReactElement<any>, container : OpaqueNode, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : void {
+    updateContainer(
+      element : ReactElement<any>,
+      container : OpaqueNode,
+      parentComponent : ?ReactComponent<any, any, any>,
+      callback: ?Function,
+      callerName: ?string
+    ) : void {
       // TODO: If this is a nested container, this won't be the root.
       const root : FiberRoot = (container.stateNode : any);
       if (callback) {
         const queue = root.callbackList ?
           root.callbackList :
           createUpdateQueue(null);
-        addCallbackToQueue(queue, callback);
+        addCallbackToQueue(queue, callback, ((callerName : any) : string));
         root.callbackList = queue;
       }
       root.pendingContext = getContextForSubtree(parentComponent);

src/renderers/shared/fiber/ReactFiberUpdateQueue.js

@@ -12,6 +12,8 @@
 
 'use strict';
 
+var invariant = require('invariant');
+
 type UpdateQueueNode = {
   partialState: any,
   callback: ?Function,
@@ -26,6 +28,29 @@ export type UpdateQueue = UpdateQueueNode & {
   tail: UpdateQueueNode
 };
 
+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) {
+  invariant(
+    !callback || typeof callback === 'function',
+    '%s(...): Expected the last optional `callback` argument to be a ' +
+    'function. Instead received: %s.',
+    callerName,
+    formatUnexpectedArgument(callback)
+  );
+}
+
 exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue {
   const queue = {
     partialState,
@@ -56,7 +81,8 @@ function addToQueue(queue : UpdateQueue, partialState : mixed) : UpdateQueue {
 
 exports.addToQueue = addToQueue;
 
-exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function) : UpdateQueue {
+exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function, callerName: string) : UpdateQueue {
+  validateCallback(callback, callerName);
   if (queue.tail.callback) {
     // If the tail already as a callback, add an empty node to queue
     addToQueue(queue, null);
@@ -115,3 +141,4 @@ exports.mergeUpdateQueue = function(queue : UpdateQueue, instance : any, prevSta
   }
   return state;
 };
+