Add callback validation to fiber-based renderers
Moved ReactFiberClassComponent validateCallback() helper function into a standalone util used by both fiber and stack implementations. Validation now happens in ReactFiberUpdateQueue so that non-DOM renderers will also benefit from it.
Brian Vaughn committed
Jan 3, 2017 at 12:10 UTC
8fbcd499bd07c8a8206f23fe0b2a1cbbc1ffaf0a
8 files changed
+63
-91
scripts/fiber/tests-failing.txt
-4
@@ -12,10 +12,6 @@ src/isomorphic/classic/__tests__/ReactContextValidator-test.js
12
src/renderers/dom/__tests__/ReactDOMProduction-test.js
13
* should throw with an error code in production
14
15
-src/renderers/dom/shared/__tests__/ReactDOM-test.js
16
-* throws in render() if the mount callback is not a function
17
-* throws in render() if the update callback is not a function
18
-
15
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
16
* should clean up input value tracking
17
* should clean up input textarea tracking
scripts/fiber/tests-passing.txt
+2
@@ -620,6 +620,8 @@ src/renderers/dom/shared/__tests__/ReactDOM-test.js
620
* should overwrite props.children with children argument
621
* should purge the DOM cache when removing nodes
622
* allow React.DOM factories to be called without warnings
623
+* throws in render() if the mount callback is not a function
624
+* throws in render() if the update callback is not a function
625
* preserves focus
626
* calls focus() on autoFocus elements after they have been mounted to the DOM
627
src/renderers/dom/shared/__tests__/ReactDOM-test.js
+6
-6
@@ -133,15 +133,15 @@ describe('ReactDOM', () => {
133
134
var myDiv = document.createElement('div');
135
expect(() => ReactDOM.render(<A />, myDiv, 'no')).toThrowError(
136
- 'ReactDOM.render(...): Expected the last optional `callback` argument ' +
136
+ 'render(...): Expected the last optional `callback` argument ' +
137
'to be a function. Instead received: string.'
138
);
139
expect(() => ReactDOM.render(<A />, myDiv, {})).toThrowError(
140
- 'ReactDOM.render(...): Expected the last optional `callback` argument ' +
140
+ 'render(...): Expected the last optional `callback` argument ' +
141
'to be a function. Instead received: Object.'
142
);
143
expect(() => ReactDOM.render(<A />, myDiv, new Foo())).toThrowError(
144
- 'ReactDOM.render(...): Expected the last optional `callback` argument ' +
144
+ 'render(...): Expected the last optional `callback` argument ' +
145
'to be a function. Instead received: Foo (keys: a, b).'
146
);
147
});
@@ -164,15 +164,15 @@ describe('ReactDOM', () => {
164
ReactDOM.render(<A />, myDiv);
165
166
expect(() => ReactDOM.render(<A />, myDiv, 'no')).toThrowError(
167
- 'ReactDOM.render(...): Expected the last optional `callback` argument ' +
167
+ 'render(...): Expected the last optional `callback` argument ' +
168
'to be a function. Instead received: string.'
169
);
170
expect(() => ReactDOM.render(<A />, myDiv, {})).toThrowError(
171
- 'ReactDOM.render(...): Expected the last optional `callback` argument ' +
171
+ 'render(...): Expected the last optional `callback` argument ' +
172
'to be a function. Instead received: Object.'
173
);
174
expect(() => ReactDOM.render(<A />, myDiv, new Foo())).toThrowError(
175
- 'ReactDOM.render(...): Expected the last optional `callback` argument ' +
175
+ 'render(...): Expected the last optional `callback` argument ' +
176
'to be a function. Instead received: Foo (keys: a, b).'
177
);
178
});
src/renderers/dom/stack/client/ReactMount.js
+2
-1
@@ -33,6 +33,7 @@ var invariant = require('invariant');
33
var setInnerHTML = require('setInnerHTML');
34
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
35
var warning = require('warning');
36
+var validateCallback = require('validateCallback');
37
38
var ATTR_NAME = DOMProperty.ID_ATTRIBUTE_NAME;
39
var ROOT_ATTR_NAME = DOMProperty.ROOT_ATTRIBUTE_NAME;
@@ -432,7 +433,7 @@ var ReactMount = {
433
},
434
435
_renderSubtreeIntoContainer: function(parentComponent, nextElement, container, callback) {
435
- ReactUpdateQueue.validateCallback(callback, 'ReactDOM.render');
436
+ validateCallback(callback, 'ReactDOM.render');
437
invariant(
438
React.isValidElement(nextElement),
439
'ReactDOM.render(): Invalid component element.%s',
src/renderers/shared/fiber/ReactFiberClassComponent.js
-34
@@ -33,31 +33,6 @@ var invariant = require('invariant');
33
34
const isArray = Array.isArray;
35
36
-function formatUnexpectedArgument(arg) {
37
- var type = typeof arg;
38
- if (type !== 'object') {
39
- return type;
40
- }
41
- var displayName = arg.constructor && arg.constructor.name || type;
42
- var keys = Object.keys(arg);
43
- if (keys.length > 0 && keys.length < 20) {
44
- return `${displayName} (keys: ${keys.join(', ')})`;
45
- }
46
- return displayName;
47
-}
48
-
49
-function validateCallback(callback, callerName) {
50
- if (typeof callback !== 'function') {
51
- invariant(
52
- false,
53
- '%s(...): Expected the last optional `callback` argument to be a ' +
54
- 'function. Instead received: %s.',
55
- callerName,
56
- formatUnexpectedArgument(callback)
57
- );
58
- }
59
-}
60
-
36
module.exports = function(
37
scheduleUpdate : (fiber : Fiber, priorityLevel : PriorityLevel) => void,
38
getPriorityContext : () => PriorityLevel,
@@ -67,27 +42,18 @@ module.exports = function(
42
const updater = {
43
isMounted,
44
enqueueSetState(instance, partialState, callback) {
70
- if (callback) {
71
- validateCallback(callback, 'setState');
72
- }
45
const fiber = ReactInstanceMap.get(instance);
46
const priorityLevel = getPriorityContext();
47
addUpdate(fiber, partialState, callback || null, priorityLevel);
48
scheduleUpdate(fiber, priorityLevel);
49
},
50
enqueueReplaceState(instance, state, callback) {
79
- if (callback) {
80
- validateCallback(callback, 'replaceState');
81
- }
51
const fiber = ReactInstanceMap.get(instance);
52
const priorityLevel = getPriorityContext();
53
addReplaceUpdate(fiber, state, callback || null, priorityLevel);
54
scheduleUpdate(fiber, priorityLevel);
55
},
56
enqueueForceUpdate(instance, callback) {
88
- if (callback) {
89
- validateCallback(callback, 'forceUpdate');
90
- }
57
const fiber = ReactInstanceMap.get(instance);
58
const priorityLevel = getPriorityContext();
59
addForceUpdate(fiber, callback || null, priorityLevel);
src/renderers/shared/fiber/ReactFiberUpdateQueue.js
+9
-19
@@ -25,6 +25,7 @@ const {
25
TaskPriority,
26
} = require('ReactPriorityLevel');
27
28
+const validateCallback = require('validateCallback');
29
const warning = require('warning');
30
31
type PartialState<State, Props> =
@@ -204,12 +205,14 @@ function findInsertionPosition(queue, update) : Update | null {
205
// we shouldn't make a copy.
206
//
207
// If the update is cloned, it returns the cloned update.
207
-function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : Update | null {
208
+function insertUpdate(fiber : Fiber, update : Update, methodName : string) : Update | null {
209
+ validateCallback(update.callback, methodName);
210
+
211
const queue1 = ensureUpdateQueue(fiber);
212
const queue2 = fiber.alternate ? ensureUpdateQueue(fiber.alternate) : null;
213
214
// Warn if an update is scheduled from inside an updater function.
212
- if (__DEV__ && typeof methodName === 'string') {
215
+ if (__DEV__) {
216
if (queue1.isProcessing || (queue2 && queue2.isProcessing)) {
217
if (methodName === 'setState') {
218
warning(
@@ -287,11 +290,7 @@ function addUpdate(
290
isTopLevelUnmount: false,
291
next: null,
292
};
290
- if (__DEV__) {
291
- insertUpdate(fiber, update, 'setState');
292
- } else {
293
- insertUpdate(fiber, update);
294
- }
293
+ insertUpdate(fiber, update, 'setState');
294
}
295
exports.addUpdate = addUpdate;
296
@@ -310,12 +309,7 @@ function addReplaceUpdate(
309
isTopLevelUnmount: false,
310
next: null,
311
};
313
-
314
- if (__DEV__) {
315
- insertUpdate(fiber, update, 'replaceState');
316
- } else {
317
- insertUpdate(fiber, update);
318
- }
312
+ insertUpdate(fiber, update, 'replaceState');
313
}
314
exports.addReplaceUpdate = addReplaceUpdate;
315
@@ -333,11 +327,7 @@ function addForceUpdate(
327
isTopLevelUnmount: false,
328
next: null,
329
};
336
- if (__DEV__) {
337
- insertUpdate(fiber, update, 'forceUpdate');
338
- } else {
339
- insertUpdate(fiber, update);
340
- }
330
+ insertUpdate(fiber, update, 'forceUpdate');
331
}
332
exports.addForceUpdate = addForceUpdate;
333
@@ -366,7 +356,7 @@ function addTopLevelUpdate(
356
isTopLevelUnmount,
357
next: null,
358
};
369
- const update2 = insertUpdate(fiber, update);
359
+ const update2 = insertUpdate(fiber, update, 'render');
360
361
if (isTopLevelUnmount) {
362
// Drop all updates that are lower-priority, so that the tree is not
src/renderers/shared/stack/reconciler/ReactUpdateQueue.js
+4
-27
@@ -16,26 +16,13 @@ var ReactInstanceMap = require('ReactInstanceMap');
16
var ReactInstrumentation = require('ReactInstrumentation');
17
var ReactUpdates = require('ReactUpdates');
18
19
-var invariant = require('invariant');
19
var warning = require('warning');
20
+var validateCallback = require('validateCallback');
21
22
function enqueueUpdate(internalInstance) {
23
ReactUpdates.enqueueUpdate(internalInstance);
24
}
25
26
-function formatUnexpectedArgument(arg) {
27
- var type = typeof arg;
28
- if (type !== 'object') {
29
- return type;
30
- }
31
- var displayName = arg.constructor && arg.constructor.name || type;
32
- var keys = Object.keys(arg);
33
- if (keys.length > 0 && keys.length < 20) {
34
- return `${displayName} (keys: ${keys.join(', ')})`;
35
- }
36
- return displayName;
37
-}
38
-
26
function getInternalInstanceReadyForUpdate(publicInstance, callerName) {
27
var internalInstance = ReactInstanceMap.get(publicInstance);
28
if (!internalInstance) {
@@ -147,7 +134,7 @@ var ReactUpdateQueue = {
134
}
135
136
if (callback) {
150
- ReactUpdateQueue.validateCallback(callback, callerName);
137
+ validateCallback(callback, callerName);
138
if (internalInstance._pendingCallbacks) {
139
internalInstance._pendingCallbacks.push(callback);
140
} else {
@@ -187,7 +174,7 @@ var ReactUpdateQueue = {
174
internalInstance._pendingReplaceState = true;
175
176
if (callback) {
190
- ReactUpdateQueue.validateCallback(callback, callerName);
177
+ validateCallback(callback, callerName);
178
if (internalInstance._pendingCallbacks) {
179
internalInstance._pendingCallbacks.push(callback);
180
} else {
@@ -235,7 +222,7 @@ var ReactUpdateQueue = {
222
queue.push(partialState);
223
224
if (callback) {
238
- ReactUpdateQueue.validateCallback(callback, callerName);
225
+ validateCallback(callback, callerName);
226
if (internalInstance._pendingCallbacks) {
227
internalInstance._pendingCallbacks.push(callback);
228
} else {
@@ -253,16 +240,6 @@ var ReactUpdateQueue = {
240
enqueueUpdate(internalInstance);
241
},
242
256
- validateCallback: function(callback, callerName) {
257
- invariant(
258
- !callback || typeof callback === 'function',
259
- '%s(...): Expected the last optional `callback` argument to be a ' +
260
- 'function. Instead received: %s.',
261
- callerName,
262
- formatUnexpectedArgument(callback)
263
- );
264
- },
265
-
243
};
244
245
module.exports = ReactUpdateQueue;
src/renderers/shared/utils/validateCallback.js
new
+40
@@ -0,0 +1,40 @@
1
+/**
2
+ * Copyright 2013-present, Facebook, Inc.
3
+ * All rights reserved.
4
+ *
5
+ * This source code is licensed under the BSD-style license found in the
6
+ * LICENSE file in the root directory of this source tree. An additional grant
7
+ * of patent rights can be found in the PATENTS file in the same directory.
8
+ *
9
+ * @providesModule validateCallback
10
+ * @flow
11
+ */
12
+
13
+'use strict';
14
+
15
+const invariant = require('invariant');
16
+
17
+function formatUnexpectedArgument(arg: any) {
18
+ let type = typeof arg;
19
+ if (type !== 'object') {
20
+ return type;
21
+ }
22
+ let displayName = arg.constructor && arg.constructor.name || type;
23
+ let keys = Object.keys(arg);
24
+ if (keys.length > 0 && keys.length < 20) {
25
+ return `${displayName} (keys: ${keys.join(', ')})`;
26
+ }
27
+ return displayName;
28
+}
29
+
30
+function validateCallback(callback: ?Function, callerName: string) {
31
+ invariant(
32
+ !callback || typeof callback === 'function',
33
+ '%s(...): Expected the last optional `callback` argument to be a ' +
34
+ 'function. Instead received: %s.',
35
+ callerName,
36
+ formatUnexpectedArgument(callback)
37
+ );
38
+}
39
+
40
+module.exports = validateCallback;