@samitouri / QOS-React / commits / d4657b8331

Add bad callback invariants to ReactDOM.render() calls

Dan Abramov committed Mar 21, 2016 at 22:31 UTC d4657b83318d59b837c04c9c4b112aa0793d8b3a
4 files changed +89 -41
src/renderers/dom/client/ReactMount.js
+1
@@ -382,6 +382,7 @@ var ReactMount = {
382 },
383
384 _renderSubtreeIntoContainer: function(parentComponent, nextElement, container, callback) {
385 + ReactUpdateQueue.validateCallback(callback, 'ReactDOM.render');
386 invariant(
387 ReactElement.isValidElement(nextElement),
388 'ReactDOM.render(): Invalid component element.%s',
src/renderers/dom/client/__tests__/ReactDOM-test.js
+59
@@ -118,4 +118,63 @@ describe('ReactDOM', function() {
118 expect(console.error.argsForCall.length).toBe(0);
119 });
120
121 + it('throws in render() if the mount callback is not a function', function() {
122 + function Foo() {
123 + this.a = 1;
124 + this.b = 2;
125 + }
126 + var A = React.createClass({
127 + getInitialState: function() {
128 + return {};
129 + },
130 + render: function() {
131 + return <div />;
132 + },
133 + });
134 +
135 + var myDiv = document.createElement('div');
136 + expect(() => ReactDOM.render(<A />, myDiv, 'no')).toThrow(
137 + 'You called `ReactDOM.render` with the last argument of type string. ' +
138 + 'When specified, its last `callback` argument must be a function.'
139 + );
140 + expect(() => ReactDOM.render(<A />, myDiv, {})).toThrow(
141 + 'You called `ReactDOM.render` with the last argument of type Object. ' +
142 + 'When specified, its last `callback` argument must be a function.'
143 + );
144 + expect(() => ReactDOM.render(<A />, myDiv, new Foo())).toThrow(
145 + 'You called `ReactDOM.render` with the last argument of type Foo (keys: a, b). ' +
146 + 'When specified, its last `callback` argument must be a function.'
147 + );
148 + });
149 +
150 + it('throws in render() if the update callback is not a function', function() {
151 + function Foo() {
152 + this.a = 1;
153 + this.b = 2;
154 + }
155 + var A = React.createClass({
156 + getInitialState: function() {
157 + return {};
158 + },
159 + render: function() {
160 + return <div />;
161 + },
162 + });
163 +
164 + var myDiv = document.createElement('div');
165 + ReactDOM.render(<A />, myDiv);
166 +
167 + expect(() => ReactDOM.render(<A />, myDiv, 'no')).toThrow(
168 + 'You called `ReactDOM.render` with the last argument of type string. ' +
169 + 'When specified, its last `callback` argument must be a function.'
170 + );
171 + expect(() => ReactDOM.render(<A />, myDiv, {})).toThrow(
172 + 'You called `ReactDOM.render` with the last argument of type Object. ' +
173 + 'When specified, its last `callback` argument must be a function.'
174 + );
175 + expect(() => ReactDOM.render(<A />, myDiv, new Foo())).toThrow(
176 + 'You called `ReactDOM.render` with the last argument of type Foo (keys: a, b). ' +
177 + 'When specified, its last `callback` argument must be a function.'
178 + );
179 + });
180 });
src/renderers/shared/reconciler/ReactUpdateQueue.js
+11 -14
@@ -120,13 +120,7 @@ var ReactUpdateQueue = {
120 * @internal
121 */
122 enqueueCallback: function(publicInstance, callback, callerName) {
123 - invariant(
124 - typeof callback === 'function',
125 - 'enqueueCallback(...): You called `%s` with the last argument of type ' +
126 - '%s. When specified, its last `callback` argument must be a function.',
127 - callerName,
128 - formatUnexpectedArgument(callback)
129 - );
123 + ReactUpdateQueue.validateCallback(callback, callerName);
124 var internalInstance = getInternalInstanceReadyForUpdate(publicInstance);
125
126 // Previously we would throw an error if we didn't have an internal
@@ -151,13 +145,6 @@ var ReactUpdateQueue = {
145 },
146
147 enqueueCallbackInternal: function(internalInstance, callback) {
154 - invariant(
155 - typeof callback === 'function',
156 - 'enqueueCallback(...): You called `setState`, `replaceState`, or ' +
157 - '`forceUpdate` with the last argument of type %s. When specified, ' +
158 - 'their last `callback` argument is expected to be a function.',
159 - formatUnexpectedArgument(callback)
160 - );
148 if (internalInstance._pendingCallbacks) {
149 internalInstance._pendingCallbacks.push(callback);
150 } else {
@@ -254,6 +241,16 @@ var ReactUpdateQueue = {
241 enqueueUpdate(internalInstance);
242 },
243
244 + validateCallback: function(callback, callerName) {
245 + invariant(
246 + !callback || typeof callback === 'function',
247 + 'You called `%s` with the last argument of type %s. ' +
248 + 'When specified, its last `callback` argument must be a function.',
249 + callerName,
250 + formatUnexpectedArgument(callback)
251 + );
252 + },
253 +
254 };
255
256 module.exports = ReactUpdateQueue;
src/renderers/shared/reconciler/__tests__/ReactUpdates-test.js
+18 -27
@@ -954,19 +954,16 @@ describe('ReactUpdates', function() {
954 var component = ReactTestUtils.renderIntoDocument(<A />);
955
956 expect(() => component.setState({}, 'no')).toThrow(
957 - 'enqueueCallback(...): You called `setState` with the last argument of ' +
958 - 'type string. When specified, its last `callback` argument must be a ' +
959 - 'function.'
957 + 'You called `setState` with the last argument of type string. ' +
958 + 'When specified, its last `callback` argument must be a function.'
959 );
960 expect(() => component.setState({}, {})).toThrow(
962 - 'enqueueCallback(...): You called `setState` with the last argument of ' +
963 - 'type Object. When specified, its last `callback` argument must be a ' +
964 - 'function.'
961 + 'You called `setState` with the last argument of type Object. ' +
962 + 'When specified, its last `callback` argument must be a function.'
963 );
964 expect(() => component.setState({}, new Foo())).toThrow(
967 - 'enqueueCallback(...): You called `setState` with the last argument of ' +
968 - 'type Foo (keys: a, b). When specified, its last `callback` argument ' +
969 - 'must be a function.'
965 + 'You called `setState` with the last argument of type Foo (keys: a, b). ' +
966 + 'When specified, its last `callback` argument must be a function.'
967 );
968 });
969
@@ -986,19 +983,16 @@ describe('ReactUpdates', function() {
983 var component = ReactTestUtils.renderIntoDocument(<A />);
984
985 expect(() => component.replaceState({}, 'no')).toThrow(
989 - 'enqueueCallback(...): You called `replaceState` with the last ' +
990 - 'argument of type string. When specified, its last `callback` argument ' +
991 - 'must be a function.'
986 + 'You called `replaceState` with the last argument of type string. ' +
987 + 'When specified, its last `callback` argument must be a function.'
988 );
989 expect(() => component.replaceState({}, {})).toThrow(
994 - 'enqueueCallback(...): You called `replaceState` with the last ' +
995 - 'argument of type Object. When specified, its last `callback` argument ' +
996 - 'must be a function.'
990 + 'You called `replaceState` with the last argument of type Object. ' +
991 + 'When specified, its last `callback` argument must be a function.'
992 );
993 expect(() => component.replaceState({}, new Foo())).toThrow(
999 - 'enqueueCallback(...): You called `replaceState` with the last ' +
1000 - 'argument of type Foo (keys: a, b). When specified, its last ' +
1001 - '`callback` argument must be a function.'
994 + 'You called `replaceState` with the last argument of type Foo (keys: a, b). ' +
995 + 'When specified, its last `callback` argument must be a function.'
996 );
997 });
998
@@ -1018,19 +1012,16 @@ describe('ReactUpdates', function() {
1012 var component = ReactTestUtils.renderIntoDocument(<A />);
1013
1014 expect(() => component.forceUpdate('no')).toThrow(
1021 - 'enqueueCallback(...): You called `forceUpdate` with the last ' +
1022 - 'argument of type string. When specified, its last `callback` argument ' +
1023 - 'must be a function.'
1015 + 'You called `forceUpdate` with the last argument of type string. ' +
1016 + 'When specified, its last `callback` argument must be a function.'
1017 );
1018 expect(() => component.forceUpdate({})).toThrow(
1026 - 'enqueueCallback(...): You called `forceUpdate` with the last ' +
1027 - 'argument of type Object. When specified, its last `callback` argument ' +
1028 - 'must be a function.'
1019 + 'You called `forceUpdate` with the last argument of type Object. ' +
1020 + 'When specified, its last `callback` argument must be a function.'
1021 );
1022 expect(() => component.forceUpdate(new Foo())).toThrow(
1031 - 'enqueueCallback(...): You called `forceUpdate` with the last ' +
1032 - 'argument of type Foo (keys: a, b). When specified, its last ' +
1033 - '`callback` argument must be a function.'
1023 + 'You called `forceUpdate` with the last argument of type Foo (keys: a, b). ' +
1024 + 'When specified, its last `callback` argument must be a function.'
1025 );
1026 });
1027 });