Schedule callback effects while merging updates
This allows us to remove the hasCallback flag.
Andrew Clark committed
Dec 9, 2016 at 11:44 UTC
6301086c354e5c296ed16f1feb9d7eae03444049
3 files changed
+16
-25
src/renderers/shared/fiber/ReactFiberBeginWork.js
+2
-12
@@ -27,7 +27,7 @@ var {
27
} = require('ReactChildFiber');
28
var {
29
hasPendingUpdate,
30
- mergeQueue,
30
+ beginUpdateQueue,
31
} = require('ReactFiberUpdateQueue');
32
var ReactTypeOfWork = require('ReactTypeOfWork');
33
var {
@@ -57,7 +57,6 @@ var {
57
} = require('ReactPriorityLevel');
58
var {
59
Update,
60
- Callback,
60
Placement,
61
ContentReset,
62
Err,
@@ -217,12 +216,6 @@ module.exports = function<T, P, I, TI, C, CX>(
216
}
217
218
// Schedule side-effects
220
- const updateQueue = workInProgress.updateQueue;
221
- if (updateQueue && updateQueue.hasCallback) {
222
- // The update queue has a callback. Schedule a callback effect.
223
- // Callbacks are scheduled regardless of whether we bail out below.
224
- workInProgress.effectTag |= Callback;
225
- }
219
if (shouldUpdate) {
220
workInProgress.effectTag |= Update;
221
} else {
@@ -537,10 +530,7 @@ module.exports = function<T, P, I, TI, C, CX>(
530
if (updateQueue) {
531
// The last three arguments are unimportant because there should be
532
// no update functions in a HostRoot's queue.
540
- mergeQueue(updateQueue, null, null, null);
541
- if (updateQueue.hasCallback) {
542
- workInProgress.effectTag |= Callback;
543
- }
533
+ beginUpdateQueue(workInProgress, updateQueue, null, null, null);
534
}
535
536
pushHostContainer(workInProgress.stateNode.containerInfo);
src/renderers/shared/fiber/ReactFiberClassComponent.js
+5
-4
@@ -24,7 +24,7 @@ var {
24
addReplaceUpdate,
25
addForceUpdate,
26
addCallback,
27
- mergeQueue,
27
+ beginUpdateQueue,
28
} = require('ReactFiberUpdateQueue');
29
var { getComponentName, isMounted } = require('ReactFiberTreeReflection');
30
var ReactInstanceMap = require('ReactInstanceMap');
@@ -78,6 +78,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
78
function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState, newContext) {
79
const updateQueue = workInProgress.updateQueue;
80
if (oldProps === null || (updateQueue && updateQueue.hasForceUpdate)) {
81
+ // If the workInProgress already has an Update effect, return true
82
return true;
83
}
84
@@ -244,7 +245,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
245
// process them now.
246
const updateQueue = workInProgress.updateQueue;
247
if (updateQueue) {
247
- instance.state = mergeQueue(updateQueue, instance, state, props);
248
+ instance.state = beginUpdateQueue(workInProgress, updateQueue, instance, state, props);
249
}
250
}
251
}
@@ -293,7 +294,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
294
// during initial mounting.
295
const newUpdateQueue = workInProgress.updateQueue;
296
if (newUpdateQueue) {
296
- newInstance.state = mergeQueue(newUpdateQueue, newInstance, newState, newProps);
297
+ newInstance.state = beginUpdateQueue(workInProgress, newUpdateQueue, newInstance, newState, newProps);
298
}
299
return true;
300
}
@@ -331,7 +332,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
332
// TODO: Previous state can be null.
333
let newState;
334
if (updateQueue) {
334
- newState = mergeQueue(updateQueue, instance, oldState, newProps);
335
+ newState = beginUpdateQueue(workInProgress, updateQueue, instance, oldState, newProps);
336
} else {
337
newState = oldState;
338
}
src/renderers/shared/fiber/ReactFiberUpdateQueue.js
+9
-9
@@ -14,6 +14,10 @@
14
15
import type { Fiber } from 'ReactFiber';
16
17
+const {
18
+ Callback: CallbackEffect,
19
+} = require('ReactTypeOfSideEffect');
20
+
21
type PartialState<State, Props> =
22
$Subtype<State> |
23
(prevState: State, props: Props) => $Subtype<State>;
@@ -40,10 +44,8 @@ export type UpdateQueue = {
44
45
// Used to implement forceUpdate. Only true if there's a merged (non-pending)
46
// force update; pending force updates do not affect this.
47
+ // TODO: We could use a ForceUpdate effect instead?
48
hasForceUpdate: boolean,
44
-
45
- // TODO: Remove this by scheduling the side-effect during the begin phase.
46
- hasCallback: boolean,
49
};
50
51
exports.createUpdateQueue = function() : UpdateQueue {
@@ -53,7 +55,6 @@ exports.createUpdateQueue = function() : UpdateQueue {
55
last: null,
56
57
hasForceUpdate: false,
56
- hasCallback: false,
58
};
59
};
60
@@ -177,15 +178,14 @@ function getStateFromUpdate(update, instance, prevState, props) {
178
}
179
}
180
180
-// TODO: Move callback effect scheduling here. Rename to beginUpdateQueue or similar.
181
-exports.mergeQueue = function(queue : UpdateQueue, instance : any, prevState : any, props : any) : any {
181
+exports.beginUpdateQueue = function(workInProgress : Fiber, queue : UpdateQueue, instance : any, prevState : any, props : any) : any {
182
// This merges the entire update queue into a single object, not just the
183
// pending updates, because the previous state and props may have changed.
184
// TODO: Would memoization be worth it?
185
186
// Reset these flags. We'll update them while looping through the queue.
187
queue.hasForceUpdate = false;
188
- queue.hasCallback = false;
188
+ workInProgress.effectTag |= CallbackEffect;
189
190
let state = prevState;
191
let dontMutatePrevState = true;
@@ -218,7 +218,7 @@ exports.mergeQueue = function(queue : UpdateQueue, instance : any, prevState : a
218
queue.hasForceUpdate = true;
219
}
220
if (update.callback) {
221
- queue.hasCallback = true;
221
+ workInProgress.effectTag |= CallbackEffect;
222
}
223
update = update.next;
224
}
@@ -236,7 +236,7 @@ exports.mergeQueue = function(queue : UpdateQueue, instance : any, prevState : a
236
};
237
238
exports.commitUpdateQueue = function(finishedWork : Fiber, queue : UpdateQueue, context : mixed) {
239
- if (queue.hasCallback) {
239
+ if (finishedWork.effectTag & CallbackEffect) {
240
// Call the callbacks on all the non-pending updates.
241
let update = queue.first;
242
while (update && update !== queue.firstPendingUpdate) {