@samitouri / QOS-React-2 / commits / 29c1e1deba

Schedule state and callback in the same batch

Fixes an issue (in both Stack and Fiber) where enqueueSetState causes a synchronous update that flushes before enqueueCallback is ever called. Now enqueueSetState et al accept an optional callback so that both are scheduled at the same time.

Andrew Clark committed Dec 20, 2016 at 13:07 UTC 29c1e1deba7bee3a230f122a558161f026063b74
10 files changed +120 -117
src/isomorphic/classic/class/ReactClass.js
+1 -4
@@ -728,10 +728,7 @@ var ReactClassMixin = {
728 * type signature and the only use case for this, is to avoid that.
729 */
730 replaceState: function(newState, callback) {
731 - this.updater.enqueueReplaceState(this, newState);
732 - if (callback) {
733 - this.updater.enqueueCallback(this, callback, 'replaceState');
734 - }
731 + this.updater.enqueueReplaceState(this, newState, callback, 'replaceState');
732 },
733
734 /**
src/isomorphic/modern/class/ReactComponent.js
+2 -8
@@ -65,10 +65,7 @@ ReactComponent.prototype.setState = function(partialState, callback) {
65 'setState(...): takes an object of state variables to update or a ' +
66 'function which returns an object of state variables.'
67 );
68 - this.updater.enqueueSetState(this, partialState);
69 - if (callback) {
70 - this.updater.enqueueCallback(this, callback, 'setState');
71 - }
68 + this.updater.enqueueSetState(this, partialState, callback, 'setState');
69 };
70
71 /**
@@ -86,10 +83,7 @@ ReactComponent.prototype.setState = function(partialState, callback) {
83 * @protected
84 */
85 ReactComponent.prototype.forceUpdate = function(callback) {
89 - this.updater.enqueueForceUpdate(this);
90 - if (callback) {
91 - this.updater.enqueueCallback(this, callback, 'forceUpdate');
92 - }
86 + this.updater.enqueueForceUpdate(this, callback, 'forceUpdate');
87 };
88
89 /**
src/renderers/shared/fiber/ReactFiberBeginWork.js
+3 -10
@@ -71,10 +71,8 @@ if (__DEV__) {
71 module.exports = function<T, P, I, TI, C, CX>(
72 config : HostConfig<T, P, I, TI, C, CX>,
73 hostContext : HostContext<C, CX>,
74 - scheduleSetState: (fiber : Fiber, partialState : any) => void,
75 - scheduleReplaceState: (fiber : Fiber, state : any) => void,
76 - scheduleForceUpdate: (fiber : Fiber) => void,
77 - scheduleUpdateCallback: (fiber : Fiber, callback : Function) => void,
74 + scheduleUpdate : (fiber : Fiber, priorityLevel : PriorityLevel) => void,
75 + getPriorityContext : () => PriorityLevel,
76 ) {
77
78 const { shouldSetTextContent } = config;
@@ -91,12 +89,7 @@ module.exports = function<T, P, I, TI, C, CX>(
89 mountClassInstance,
90 resumeMountClassInstance,
91 updateClassInstance,
94 - } = ReactFiberClassComponent(
95 - scheduleSetState,
96 - scheduleReplaceState,
97 - scheduleForceUpdate,
98 - scheduleUpdateCallback
99 - );
92 + } = ReactFiberClassComponent(scheduleUpdate, getPriorityContext);
93
94 function markChildAsProgressed(current, workInProgress, priorityLevel) {
95 // We now have clones. Let's store them as the currently progressed work.
src/renderers/shared/fiber/ReactFiberClassComponent.js
+21 -11
@@ -19,6 +19,10 @@ var {
19 getMaskedContext,
20 } = require('ReactFiberContext');
21 var {
22 + addUpdate,
23 + addReplaceUpdate,
24 + addForceUpdate,
25 + addCallback,
26 beginUpdateQueue,
27 } = require('ReactFiberUpdateQueue');
28 var { hasContextChanged } = require('ReactFiberContext');
@@ -31,30 +35,36 @@ var invariant = require('invariant');
35 const isArray = Array.isArray;
36
37 module.exports = function(
34 - scheduleSetState: (fiber : Fiber, partialState : any) => void,
35 - scheduleReplaceState: (fiber : Fiber, state : any) => void,
36 - scheduleForceUpdate: (fiber : Fiber) => void,
37 - scheduleUpdateCallback: (fiber : Fiber, callback : Function) => void,
38 + scheduleUpdate : (fiber : Fiber, priorityLevel : PriorityLevel) => void,
39 + getPriorityContext : () => PriorityLevel,
40 ) {
41
42 // Class component state updater
43 const updater = {
44 isMounted,
43 - enqueueSetState(instance, partialState) {
45 + enqueueSetState(instance, partialState, callback) {
46 const fiber = ReactInstanceMap.get(instance);
45 - scheduleSetState(fiber, partialState);
47 + const priorityLevel = getPriorityContext();
48 + addUpdate(fiber, partialState, callback || null, priorityLevel);
49 + scheduleUpdate(fiber, priorityLevel);
50 },
47 - enqueueReplaceState(instance, state) {
51 + enqueueReplaceState(instance, state, callback) {
52 const fiber = ReactInstanceMap.get(instance);
49 - scheduleReplaceState(fiber, state);
53 + const priorityLevel = getPriorityContext();
54 + addReplaceUpdate(fiber, state, callback || null, priorityLevel);
55 + scheduleUpdate(fiber, priorityLevel);
56 },
51 - enqueueForceUpdate(instance) {
57 + enqueueForceUpdate(instance, callback) {
58 const fiber = ReactInstanceMap.get(instance);
53 - scheduleForceUpdate(fiber);
59 + const priorityLevel = getPriorityContext();
60 + addForceUpdate(fiber, callback || null, priorityLevel);
61 + scheduleUpdate(fiber, priorityLevel);
62 },
63 enqueueCallback(instance, callback) {
64 const fiber = ReactInstanceMap.get(instance);
57 - scheduleUpdateCallback(fiber, callback);
65 + const priorityLevel = getPriorityContext();
66 + addCallback(fiber, callback || null, priorityLevel);
67 + scheduleUpdate(fiber, priorityLevel);
68 },
69 };
70
src/renderers/shared/fiber/ReactFiberReconciler.js
+15 -10
@@ -17,6 +17,10 @@ import type { FiberRoot } from 'ReactFiberRoot';
17 import type { PriorityLevel } from 'ReactPriorityLevel';
18 import type { ReactNodeList } from 'ReactTypes';
19
20 +var {
21 + addTopLevelUpdate,
22 +} = require('ReactFiberUpdateQueue');
23 +
24 var {
25 findCurrentUnmaskedContext,
26 isContextProvider,
@@ -98,14 +102,21 @@ getContextForSubtree._injectFiber(function(fiber : Fiber) {
102 module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C, CX>) : Reconciler<C, I, TI> {
103
104 var {
101 - scheduleTopLevelSetState,
102 - scheduleUpdateCallback,
105 + scheduleUpdate,
106 + getPriorityContext,
107 performWithPriority,
108 batchedUpdates,
109 syncUpdates,
110 deferredUpdates,
111 } = ReactFiberScheduler(config);
112
113 + function scheduleTopLevelUpdate(current : Fiber, element : ReactNodeList, callback : ?Function) {
114 + const priorityLevel = getPriorityContext();
115 + const nextState = { element };
116 + addTopLevelUpdate(current, nextState, callback || null, priorityLevel);
117 + scheduleUpdate(current, priorityLevel);
118 + }
119 +
120 return {
121
122 mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : OpaqueNode {
@@ -113,10 +124,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
124 const root = createFiberRoot(containerInfo, context);
125 const current = root.current;
126
116 - scheduleTopLevelSetState(current, { element });
117 - if (callback) {
118 - scheduleUpdateCallback(current, callback);
119 - }
127 + scheduleTopLevelUpdate(current, element, callback);
128
129 if (__DEV__ && ReactFiberInstrumentation.debugTool) {
130 ReactFiberInstrumentation.debugTool.onMountContainer(root);
@@ -135,10 +143,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
143
144 root.pendingContext = getContextForSubtree(parentComponent);
145
138 - scheduleTopLevelSetState(current, { element });
139 - if (callback) {
140 - scheduleUpdateCallback(current, callback);
141 - }
146 + scheduleTopLevelUpdate(current, element, callback);
147
148 if (__DEV__) {
149 if (ReactFiberInstrumentation.debugTool) {
src/renderers/shared/fiber/ReactFiberScheduler.js
+12 -38
@@ -55,11 +55,6 @@ var {
55
56 var {
57 getPendingPriority,
58 - addUpdate,
59 - addReplaceUpdate,
60 - addForceUpdate,
61 - addCallback,
62 - addTopLevelUpdate,
58 } = require('ReactFiberUpdateQueue');
59
60 var {
@@ -79,10 +74,8 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
74 const { beginWork, beginFailedWork } = ReactFiberBeginWork(
75 config,
76 hostContext,
82 - scheduleSetState,
83 - scheduleReplaceState,
84 - scheduleForceUpdate,
85 - scheduleUpdateCallback,
77 + scheduleUpdate,
78 + getPriorityContext,
79 );
80 const { completeWork } = ReactFiberCompleteWork(config, hostContext);
81 const {
@@ -986,7 +979,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
979 }
980 }
981
989 - function scheduleUpdateAtPriority(fiber : Fiber, priorityLevel : PriorityLevel) {
982 + function scheduleUpdate(fiber : Fiber, priorityLevel : PriorityLevel) {
983 // If we're in a batch, downgrade sync priority to task priority
984 if (priorityLevel === SynchronousPriority && isPerformingWork) {
985 priorityLevel = TaskPriority;
@@ -1049,34 +1042,15 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
1042 }
1043 }
1044
1052 - function scheduleErrorRecovery(fiber : Fiber) {
1053 - scheduleUpdateAtPriority(fiber, TaskPriority);
1054 - }
1055 -
1056 - function scheduleSetState(fiber : Fiber, partialState : any) {
1057 - addUpdate(fiber, partialState, priorityContext);
1058 - scheduleUpdateAtPriority(fiber, priorityContext);
1059 - }
1060 -
1061 - function scheduleReplaceState(fiber : Fiber, state : any) {
1062 - addReplaceUpdate(fiber, state, priorityContext);
1063 - scheduleUpdateAtPriority(fiber, priorityContext);
1064 - }
1065 -
1066 - function scheduleForceUpdate(fiber : Fiber) {
1067 - addForceUpdate(fiber, priorityContext);
1068 - scheduleUpdateAtPriority(fiber, priorityContext);
1069 - }
1070 -
1071 - function scheduleUpdateCallback(fiber : Fiber, callback : Function) {
1072 - addCallback(fiber, callback, priorityContext);
1073 - scheduleUpdateAtPriority(fiber, priorityContext);
1045 + function getPriorityContext() : PriorityLevel {
1046 + if (priorityContext === SynchronousPriority && isPerformingWork) {
1047 + return TaskPriority;
1048 + }
1049 + return priorityContext;
1050 }
1051
1076 - // TODO: This indirection will be removed as part of #8585
1077 - function scheduleTopLevelSetState(fiber : Fiber, partialState : any) {
1078 - addTopLevelUpdate(fiber, partialState, priorityContext);
1079 - scheduleUpdateAtPriority(fiber, priorityContext);
1052 + function scheduleErrorRecovery(fiber : Fiber) {
1053 + scheduleUpdate(fiber, TaskPriority);
1054 }
1055
1056 function performWithPriority(priorityLevel : PriorityLevel, fn : Function) {
@@ -1126,8 +1100,8 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
1100 }
1101
1102 return {
1129 - scheduleTopLevelSetState: scheduleTopLevelSetState,
1130 - scheduleUpdateCallback: scheduleUpdateCallback,
1103 + scheduleUpdate: scheduleUpdate,
1104 + getPriorityContext: getPriorityContext,
1105 performWithPriority: performWithPriority,
1106 batchedUpdates: batchedUpdates,
1107 syncUpdates: syncUpdates,
src/renderers/shared/fiber/ReactFiberUpdateQueue.js
+21 -7
@@ -284,12 +284,13 @@ function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : Up
284 function addUpdate(
285 fiber : Fiber,
286 partialState : PartialState<any, any> | null,
287 + callback : Callback | null,
288 priorityLevel : PriorityLevel
289 ) : void {
290 const update = {
291 priorityLevel,
292 partialState,
292 - callback: null,
293 + callback,
294 isReplace: false,
295 isForced: false,
296 isTopLevelUnmount: false,
@@ -306,12 +307,13 @@ exports.addUpdate = addUpdate;
307 function addReplaceUpdate(
308 fiber : Fiber,
309 state : any | null,
310 + callback : Callback | null,
311 priorityLevel : PriorityLevel
312 ) : void {
313 const update = {
314 priorityLevel,
315 partialState: state,
314 - callback: null,
316 + callback,
317 isReplace: true,
318 isForced: false,
319 isTopLevelUnmount: false,
@@ -326,11 +328,15 @@ function addReplaceUpdate(
328 }
329 exports.addReplaceUpdate = addReplaceUpdate;
330
329 -function addForceUpdate(fiber : Fiber, priorityLevel : PriorityLevel) : void {
331 +function addForceUpdate(
332 + fiber : Fiber,
333 + callback : Callback | null,
334 + priorityLevel : PriorityLevel
335 +) : void {
336 const update = {
337 priorityLevel,
338 partialState: null,
333 - callback: null,
339 + callback,
340 isReplace: false,
341 isForced: true,
342 isTopLevelUnmount: false,
@@ -345,7 +351,11 @@ function addForceUpdate(fiber : Fiber, priorityLevel : PriorityLevel) : void {
351 exports.addForceUpdate = addForceUpdate;
352
353
348 -function addCallback(fiber : Fiber, callback: Callback, priorityLevel : PriorityLevel) : void {
354 +function addCallback(
355 + fiber : Fiber,
356 + callback: Callback,
357 + priorityLevel : PriorityLevel
358 +) : void {
359 const update : Update = {
360 priorityLevel,
361 partialState: null,
@@ -367,14 +377,18 @@ exports.getPendingPriority = getPendingPriority;
377 function addTopLevelUpdate(
378 fiber : Fiber,
379 partialState : PartialState<any, any>,
380 + callback : Callback | null,
381 priorityLevel : PriorityLevel
382 ) : void {
372 - const isTopLevelUnmount = partialState === null;
383 + const isTopLevelUnmount = Boolean(
384 + partialState &&
385 + partialState.element === null
386 + );
387
388 const update = {
389 priorityLevel,
390 partialState,
377 - callback: null,
391 + callback,
392 isReplace: false,
393 isForced: false,
394 isTopLevelUnmount,
src/renderers/shared/shared/__tests__/ReactCompositeComponentState-test.js
+3 -22
@@ -171,20 +171,8 @@ describe('ReactCompositeComponent-state', () => {
171 ['componentDidMount-end', 'orange'],
172 ['setState-sunrise', 'orange'],
173 ['setState-orange', 'orange'],
174 - ];
175 -
176 - // In Fiber, the initial callback is not enqueued until after any work
177 - // scheduled by lifecycles has flushed (same semantics as a regular setState
178 - // callback outside of a batch). In Stack, the initial render is scheduled
179 - // inside of batchedUpdates, so the callback gets flushed right after
180 - // componentDidMount.
181 - // TODO: We should fix this, in both Stack and Fiber, so that the behavior
182 - // is consistent regardless of whether you're in a batch.
183 - if (!ReactDOMFeatureFlags.useFiber) {
184 - expected.push(['initial-callback', 'orange']);
185 - }
186 -
187 - expected.push(['shouldComponentUpdate-currentState', 'orange'],
174 + ['initial-callback', 'orange'],
175 + ['shouldComponentUpdate-currentState', 'orange'],
176 ['shouldComponentUpdate-nextState', 'yellow'],
177 ['componentWillUpdate-currentState', 'orange'],
178 ['componentWillUpdate-nextState', 'yellow'],
@@ -192,18 +180,11 @@ describe('ReactCompositeComponent-state', () => {
180 ['componentDidUpdate-currentState', 'yellow'],
181 ['componentDidUpdate-prevState', 'orange'],
182 ['setState-yellow', 'yellow'],
195 - );
196 -
197 - if (ReactDOMFeatureFlags.useFiber) {
198 - expected.push(['initial-callback', 'yellow']);
199 - }
200 -
201 - expected.push(
183 ['componentWillReceiveProps-start', 'yellow'],
184 // setState({color:'green'}) only enqueues a pending state.
185 ['componentWillReceiveProps-end', 'yellow'],
186 // pending state queue is processed
206 - );
187 + ];
188
189 if (ReactDOMFeatureFlags.useFiber) {
190 // In Stack, this is never called because replaceState drops all updates
src/renderers/shared/shared/__tests__/ReactUpdates-test.js
+6 -4
@@ -648,16 +648,18 @@ describe('ReactUpdates', () => {
648 'Inner-render-1-0',
649 'Inner-didUpdate-1-0',
650 'Outer-didUpdate-1',
651 + // Happens in a batch, so don't re-render yet
652 'Inner-setState-1',
652 - 'Inner-render-1-1',
653 - 'Inner-didUpdate-1-1',
654 - 'Inner-callback-1',
653 'Outer-callback-1',
654
657 - 'Outer-setState-2',
655 + // Happens in a batch
656 + 'Outer-setState-2',
657 +
658 + // Flush batched updates all at once
659 'Outer-render-2',
660 'Inner-render-2-1',
661 'Inner-didUpdate-2-1',
662 + 'Inner-callback-1',
663 'Outer-didUpdate-2',
664 'Inner-setState-2',
665 'Outer-callback-2',
src/renderers/shared/stack/reconciler/ReactUpdateQueue.js
+36 -3
@@ -166,9 +166,11 @@ var ReactUpdateQueue = {
166 * `componentWillUpdate` and `componentDidUpdate`.
167 *
168 * @param {ReactClass} publicInstance The instance that should rerender.
169 + * @param {?function} callback Called after component is updated.
170 + * @param {?string} Name of the calling function in the public API.
171 * @internal
172 */
171 - enqueueForceUpdate: function(publicInstance) {
173 + enqueueForceUpdate: function(publicInstance, callback, callerName) {
174 var internalInstance = getInternalInstanceReadyForUpdate(
175 publicInstance,
176 'forceUpdate'
@@ -178,6 +180,15 @@ var ReactUpdateQueue = {
180 return;
181 }
182
183 + if (callback) {
184 + ReactUpdateQueue.validateCallback(callback, callerName);
185 + if (internalInstance._pendingCallbacks) {
186 + internalInstance._pendingCallbacks.push(callback);
187 + } else {
188 + internalInstance._pendingCallbacks = [callback];
189 + }
190 + }
191 +
192 internalInstance._pendingForceUpdate = true;
193
194 enqueueUpdate(internalInstance);
@@ -192,9 +203,11 @@ var ReactUpdateQueue = {
203 *
204 * @param {ReactClass} publicInstance The instance that should rerender.
205 * @param {object} completeState Next state.
206 + * @param {?function} callback Called after state is updated.
207 + * @param {?string} Name of the calling function in the public API.
208 * @internal
209 */
197 - enqueueReplaceState: function(publicInstance, completeState) {
210 + enqueueReplaceState: function(publicInstance, completeState, callback, callerName) {
211 var internalInstance = getInternalInstanceReadyForUpdate(
212 publicInstance,
213 'replaceState'
@@ -207,6 +220,15 @@ var ReactUpdateQueue = {
220 internalInstance._pendingStateQueue = [completeState];
221 internalInstance._pendingReplaceState = true;
222
223 + if (callback) {
224 + ReactUpdateQueue.validateCallback(callback, callerName);
225 + if (internalInstance._pendingCallbacks) {
226 + internalInstance._pendingCallbacks.push(callback);
227 + } else {
228 + internalInstance._pendingCallbacks = [callback];
229 + }
230 + }
231 +
232 enqueueUpdate(internalInstance);
233 },
234
@@ -218,9 +240,11 @@ var ReactUpdateQueue = {
240 *
241 * @param {ReactClass} publicInstance The instance that should rerender.
242 * @param {object} partialState Next partial state to be merged with state.
243 + * @param {?function} callback Called after state is updated.
244 + * @param {?string} Name of the calling function in the public API.
245 * @internal
246 */
223 - enqueueSetState: function(publicInstance, partialState) {
247 + enqueueSetState: function(publicInstance, partialState, callback, callerName) {
248 if (__DEV__) {
249 ReactInstrumentation.debugTool.onSetState();
250 warning(
@@ -244,6 +268,15 @@ var ReactUpdateQueue = {
268 (internalInstance._pendingStateQueue = []);
269 queue.push(partialState);
270
271 + if (callback) {
272 + ReactUpdateQueue.validateCallback(callback, callerName);
273 + if (internalInstance._pendingCallbacks) {
274 + internalInstance._pendingCallbacks.push(callback);
275 + } else {
276 + internalInstance._pendingCallbacks = [callback];
277 + }
278 + }
279 +
280 enqueueUpdate(internalInstance);
281 },
282