Merge pull request #6310 from gaearon/setstate-warning

Add more specific error messages for bad callback in setState, replaceState, and ReactDOM.render

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');
     }
   },
 

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');
   }
 };
 

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',

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 <div />;
+      },
+    });
+
+    var myDiv = document.createElement('div');
+    expect(() => ReactDOM.render(<A />, myDiv, 'no')).toThrow(
+      'ReactDOM.render(...): Expected the last optional `callback` argument ' +
+      'to be a function. Instead received: string.'
+    );
+    expect(() => ReactDOM.render(<A />, myDiv, {})).toThrow(
+      'ReactDOM.render(...): Expected the last optional `callback` argument ' +
+      'to be a function. Instead received: Object.'
+    );
+    expect(() => ReactDOM.render(<A />, 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 <div />;
+      },
+    });
+
+    var myDiv = document.createElement('div');
+    ReactDOM.render(<A />, myDiv);
+
+    expect(() => ReactDOM.render(<A />, myDiv, 'no')).toThrow(
+      'ReactDOM.render(...): Expected the last optional `callback` argument ' +
+      'to be a function. Instead received: string.'
+    );
+    expect(() => ReactDOM.render(<A />, myDiv, {})).toThrow(
+      'ReactDOM.render(...): Expected the last optional `callback` argument ' +
+      'to be a function. Instead received: Object.'
+    );
+    expect(() => ReactDOM.render(<A />, myDiv, new Foo())).toThrow(
+      'ReactDOM.render(...): Expected the last optional `callback` argument ' +
+      'to be a function. Instead received: Foo (keys: a, 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;

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 <div />;
+      },
+    });
+    var component = ReactTestUtils.renderIntoDocument(<A />);
+
+    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 <div />;
+      },
+    });
+    var component = ReactTestUtils.renderIntoDocument(<A />);
+
+    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 <div />;
+      },
+    });
+    var component = ReactTestUtils.renderIntoDocument(<A />);
+
+    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).'
+    );
+  });
 });