Skip to content

Commit 64ad409

Browse files
koba04koba04
committed
Fix Fiber tests for update callback warnings
1 parent e24ec4a commit 64ad409

File tree

7 files changed

+81
-24
lines changed

7 files changed

+81
-24
lines changed

‎scripts/fiber/tests-failing.txt‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,6 @@ src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js
4545
src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
4646
* should control a value in reentrant events
4747

48-
src/renderers/dom/stack/client/__tests__/ReactDOM-test.js
49-
* throws in render() if the mount callback is not a function
50-
* throws in render() if the update callback is not a function
51-
5248
src/renderers/dom/stack/client/__tests__/ReactMount-test.js
5349
* throws when given a non-node
5450
* throws when given a string
@@ -111,9 +107,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactStatelessComponent-test.js
111107
src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js
112108
* should queue mount-ready handlers across different roots
113109
* marks top-level updates
114-
* throws in setState if the update callback is not a function
115-
* throws in replaceState if the update callback is not a function
116-
* throws in forceUpdate if the update callback is not a function
117110

118111
src/renderers/shared/stack/reconciler/__tests__/refs-test.js
119112
* Should increase refs with an increase in divs

‎scripts/fiber/tests-passing.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,8 @@ src/renderers/dom/stack/client/__tests__/ReactDOM-test.js
934934
* should overwrite props.children with children argument
935935
* should purge the DOM cache when removing nodes
936936
* allow React.DOM factories to be called without warnings
937+
* throws in render() if the mount callback is not a function
938+
* throws in render() if the update callback is not a function
937939
* preserves focus
938940

939941
src/renderers/dom/stack/client/__tests__/ReactMount-test.js
@@ -1459,6 +1461,9 @@ src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js
14591461
* should queue updates from during mount
14601462
* calls componentWillReceiveProps setState callback properly
14611463
* does not call render after a component as been deleted
1464+
* throws in setState if the update callback is not a function
1465+
* throws in replaceState if the update callback is not a function
1466+
* throws in forceUpdate if the update callback is not a function
14621467
* does not update one component twice in a batch (#2410)
14631468
* does not update one component twice in a batch (#6371)
14641469
* unstable_batchedUpdates should return value from a callback

‎src/renderers/dom/fiber/ReactDOMFiber.js‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,13 @@ function warnAboutUnstableUse() {
165165
warned = true;
166166
}
167167

168-
function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any, any>, element : ReactElement<any>, containerNode : DOMContainerElement | Document, callback: ?Function) {
168+
function renderSubtreeIntoContainer(
169+
parentComponent : ?ReactComponent<any, any, any>,
170+
element : ReactElement<any>,
171+
containerNode : DOMContainerElement | Document,
172+
callback: ?Function,
173+
callerName: ?string
174+
) {
169175
let container : DOMContainerElement =
170176
containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any);
171177
let root;
@@ -174,9 +180,10 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any,
174180
while (container.lastChild) {
175181
container.removeChild(container.lastChild);
176182
}
177-
root = container._reactRootContainer = DOMRenderer.mountContainer(element, container, parentComponent, callback);
183+
root = container._reactRootContainer =
184+
DOMRenderer.mountContainer(element, container, parentComponent, callback, callerName);
178185
} else {
179-
DOMRenderer.updateContainer(element, root = container._reactRootContainer, parentComponent, callback);
186+
DOMRenderer.updateContainer(element, root = container._reactRootContainer, parentComponent, callback, callerName);
180187
}
181188
return DOMRenderer.getPublicRootInstance(root);
182189
}
@@ -185,15 +192,22 @@ var ReactDOM = {
185192

186193
render(element : ReactElement<any>, container : DOMContainerElement, callback: ?Function) {
187194
warnAboutUnstableUse();
188-
return renderSubtreeIntoContainer(null, element, container, callback);
195+
var callerName = 'ReactDOM.render';
196+
return renderSubtreeIntoContainer(null, element, container, callback, callerName);
189197
},
190198

191-
unstable_renderSubtreeIntoContainer(parentComponent : ReactComponent<any, any, any>, element : ReactElement<any>, containerNode : DOMContainerElement | Document, callback: ?Function) {
199+
unstable_renderSubtreeIntoContainer(
200+
parentComponent : ReactComponent<any, any, any>,
201+
element : ReactElement<any>,
202+
containerNode : DOMContainerElement | Document,
203+
callback: ?Function
204+
) {
192205
invariant(
193206
parentComponent != null && ReactInstanceMap.has(parentComponent),
194207
'parentComponent must be a valid React Component'
195208
);
196-
return renderSubtreeIntoContainer(parentComponent, element, containerNode, callback);
209+
var callerName = 'ReactDOM.unstable_renderSubtreeIntoContainer';
210+
return renderSubtreeIntoContainer(parentComponent, element, containerNode, callback, callerName);
197211
},
198212

199213
unmountComponentAtNode(container : DOMContainerElement) {

‎src/renderers/noop/ReactNoop.js‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,19 +156,25 @@ var ReactNoop = {
156156

157157
// Shortcut for testing a single root
158158
render(element : ReactElement<any>, callback: ?Function) {
159-
ReactNoop.renderToRootWithID(element, DEFAULT_ROOT_ID, callback);
159+
var callerName = 'ReactNoop.render';
160+
ReactNoop.renderToRootWithID(element, DEFAULT_ROOT_ID, callback, callerName);
160161
},
161162

162-
renderToRootWithID(element : ReactElement<any>, rootID : string, callback : ?Function) {
163+
renderToRootWithID(
164+
element : ReactElement<any>,
165+
rootID : string,
166+
callback : ?Function,
167+
callerName: ?string = 'ReactNoop.renderToRootWithID'
168+
) {
163169
if (!roots.has(rootID)) {
164170
const container = { rootID: rootID, children: [] };
165171
rootContainers.set(rootID, container);
166-
const root = NoopRenderer.mountContainer(element, container, null, callback);
172+
const root = NoopRenderer.mountContainer(element, container, null, callback, callerName);
167173
roots.set(rootID, root);
168174
} else {
169175
const root = roots.get(rootID);
170176
if (root) {
171-
NoopRenderer.updateContainer(element, root, null, callback);
177+
NoopRenderer.updateContainer(element, root, null, callback, callerName);
172178
}
173179
}
174180
},

‎src/renderers/shared/fiber/ReactFiberClassComponent.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
6666
updateQueue.isForced = true;
6767
scheduleUpdateQueue(fiber, updateQueue);
6868
},
69-
enqueueCallback(instance, callback) {
69+
enqueueCallback(instance, callback, callerName) {
7070
const fiber = ReactInstanceMap.get(instance);
7171
let updateQueue = fiber.updateQueue ?
7272
fiber.updateQueue :
7373
createUpdateQueue(null);
74-
addCallbackToQueue(updateQueue, callback);
74+
addCallbackToQueue(updateQueue, callback, callerName);
7575
scheduleUpdateQueue(fiber, updateQueue);
7676
},
7777
};

‎src/renderers/shared/fiber/ReactFiberReconciler.js‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,19 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) :
101101

102102
return {
103103

104-
mountContainer(element : ReactElement<any>, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : OpaqueNode {
104+
mountContainer(
105+
element : ReactElement<any>,
106+
containerInfo : C,
107+
parentComponent : ?ReactComponent<any, any, any>,
108+
callback: ?Function,
109+
callerName: ?string
110+
) : OpaqueNode {
105111
const context = getContextForSubtree(parentComponent);
106112
const root = createFiberRoot(containerInfo, context);
107113
const container = root.current;
108114
if (callback) {
109115
const queue = createUpdateQueue(null);
110-
addCallbackToQueue(queue, callback);
116+
addCallbackToQueue(queue, callback, ((callerName : any) : string));
111117
root.callbackList = queue;
112118
}
113119
// 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>) :
127133
return container;
128134
},
129135

130-
updateContainer(element : ReactElement<any>, container : OpaqueNode, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : void {
136+
updateContainer(
137+
element : ReactElement<any>,
138+
container : OpaqueNode,
139+
parentComponent : ?ReactComponent<any, any, any>,
140+
callback: ?Function,
141+
callerName: ?string
142+
) : void {
131143
// TODO: If this is a nested container, this won't be the root.
132144
const root : FiberRoot = (container.stateNode : any);
133145
if (callback) {
134146
const queue = root.callbackList ?
135147
root.callbackList :
136148
createUpdateQueue(null);
137-
addCallbackToQueue(queue, callback);
149+
addCallbackToQueue(queue, callback, ((callerName : any) : string));
138150
root.callbackList = queue;
139151
}
140152
root.pendingContext = getContextForSubtree(parentComponent);

‎src/renderers/shared/fiber/ReactFiberUpdateQueue.js‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
'use strict';
1414

15+
var invariant = require('invariant');
16+
1517
type UpdateQueueNode = {
1618
partialState: any,
1719
callback: ?Function,
@@ -26,6 +28,29 @@ export type UpdateQueue = UpdateQueueNode & {
2628
tail: UpdateQueueNode
2729
};
2830

31+
function formatUnexpectedArgument(arg) {
32+
var type = typeof arg;
33+
if (type !== 'object') {
34+
return type;
35+
}
36+
var displayName = arg.constructor && arg.constructor.name || type;
37+
var keys = Object.keys(arg);
38+
if (keys.length > 0 && keys.length < 20) {
39+
return `${displayName} (keys: ${keys.join(', ')})`;
40+
}
41+
return displayName;
42+
}
43+
44+
function validateCallback(callback, callerName) {
45+
invariant(
46+
!callback || typeof callback === 'function',
47+
'%s(...): Expected the last optional `callback` argument to be a ' +
48+
'function. Instead received: %s.',
49+
callerName,
50+
formatUnexpectedArgument(callback)
51+
);
52+
}
53+
2954
exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue {
3055
const queue = {
3156
partialState,
@@ -56,7 +81,8 @@ function addToQueue(queue : UpdateQueue, partialState : mixed) : UpdateQueue {
5681

5782
exports.addToQueue = addToQueue;
5883

59-
exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function) : UpdateQueue {
84+
exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function, callerName: string) : UpdateQueue {
85+
validateCallback(callback, callerName);
6086
if (queue.tail.callback) {
6187
// If the tail already as a callback, add an empty node to queue
6288
addToQueue(queue, null);
@@ -115,3 +141,4 @@ exports.mergeUpdateQueue = function(queue : UpdateQueue, instance : any, prevSta
115141
}
116142
return state;
117143
};
144+

0 commit comments

Comments
 (0)