Refactor Update Queues to Fix Rebasing Bug
Fixes a bug related to rebasing updates. Once an update has committed, it should never un-commit, even if interrupted by a higher priority update. The fix includes a refactor of how update queues work. This commit is a combination of two PRs: - #17483 by @sebmarkbage refactors the hook update queue - #17510 by @acdlite refactors the class and root update queue Landing one without the other would cause state updates to sometimes be inconsistent across components, so I've combined them into a single commit in case they need to be reverted. Co-authored-by: Sebastian Markbåge <sema@fb.com> Co-authored-by: Andrew Clark <git@andrewclark.io>
Andrew Clark committed
Dec 9, 2019 at 13:19 UTC
b617db3d966f678eb0b4aac6d96f7967b37a9e91
5 files changed
+479
-344
packages/react-noop-renderer/src/createReactNoop.js
+24
-11
@@ -1142,20 +1142,33 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
1142
1143
function logUpdateQueue(updateQueue: UpdateQueue<mixed>, depth) {
1144
log(' '.repeat(depth + 1) + 'QUEUED UPDATES');
1145
- const firstUpdate = updateQueue.firstUpdate;
1146
- if (!firstUpdate) {
1145
+ const last = updateQueue.baseQueue;
1146
+ if (last === null) {
1147
return;
1148
}
1149
+ const first = last.next;
1150
+ let update = first;
1151
+ if (update !== null) {
1152
+ do {
1153
+ log(
1154
+ ' '.repeat(depth + 1) + '~',
1155
+ '[' + update.expirationTime + ']',
1156
+ );
1157
+ } while (update !== null && update !== first);
1158
+ }
1159
1150
- log(
1151
- ' '.repeat(depth + 1) + '~',
1152
- '[' + firstUpdate.expirationTime + ']',
1153
- );
1154
- while (firstUpdate.next) {
1155
- log(
1156
- ' '.repeat(depth + 1) + '~',
1157
- '[' + firstUpdate.expirationTime + ']',
1158
- );
1160
+ const lastPending = updateQueue.shared.pending;
1161
+ if (lastPending !== null) {
1162
+ const firstPending = lastPending.next;
1163
+ let pendingUpdate = firstPending;
1164
+ if (pendingUpdate !== null) {
1165
+ do {
1166
+ log(
1167
+ ' '.repeat(depth + 1) + '~',
1168
+ '[' + pendingUpdate.expirationTime + ']',
1169
+ );
1170
+ } while (pendingUpdate !== null && pendingUpdate !== firstPending);
1171
+ }
1172
}
1173
}
1174
packages/react-reconciler/src/ReactFiberHooks.js
+70
-49
@@ -20,7 +20,7 @@ import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
20
21
import ReactSharedInternals from 'shared/ReactSharedInternals';
22
23
-import {NoWork} from './ReactFiberExpirationTime';
23
+import {NoWork, Sync} from './ReactFiberExpirationTime';
24
import {readContext} from './ReactFiberNewContext';
25
import {createResponderListener} from './ReactFiberEvents';
26
import {
@@ -108,13 +108,13 @@ type Update<S, A> = {
108
action: A,
109
eagerReducer: ((S, A) => S) | null,
110
eagerState: S | null,
111
- next: Update<S, A> | null,
111
+ next: Update<S, A>,
112
113
priority?: ReactPriorityLevel,
114
};
115
116
type UpdateQueue<S, A> = {
117
- last: Update<S, A> | null,
117
+ pending: Update<S, A> | null,
118
dispatch: (A => mixed) | null,
119
lastRenderedReducer: ((S, A) => S) | null,
120
lastRenderedState: S | null,
@@ -144,7 +144,7 @@ export type Hook = {
144
memoizedState: any,
145
146
baseState: any,
147
- baseUpdate: Update<any, any> | null,
147
+ baseQueue: Update<any, any> | null,
148
queue: UpdateQueue<any, any> | null,
149
150
next: Hook | null,
@@ -544,8 +544,8 @@ function mountWorkInProgressHook(): Hook {
544
memoizedState: null,
545
546
baseState: null,
547
+ baseQueue: null,
548
queue: null,
548
- baseUpdate: null,
549
550
next: null,
551
};
@@ -604,8 +604,8 @@ function updateWorkInProgressHook(): Hook {
604
memoizedState: currentHook.memoizedState,
605
606
baseState: currentHook.baseState,
607
+ baseQueue: currentHook.baseQueue,
608
queue: currentHook.queue,
608
- baseUpdate: currentHook.baseUpdate,
609
610
next: null,
611
};
@@ -645,7 +645,7 @@ function mountReducer<S, I, A>(
645
}
646
hook.memoizedState = hook.baseState = initialState;
647
const queue = (hook.queue = {
648
- last: null,
648
+ pending: null,
649
dispatch: null,
650
lastRenderedReducer: reducer,
651
lastRenderedState: (initialState: any),
@@ -703,7 +703,7 @@ function updateReducer<S, I, A>(
703
// the base state unless the queue is empty.
704
// TODO: Not sure if this is the desired semantics, but it's what we
705
// do for gDSFP. I can't remember why.
706
- if (hook.baseUpdate === queue.last) {
706
+ if (hook.baseQueue === null) {
707
hook.baseState = newState;
708
}
709
@@ -715,42 +715,55 @@ function updateReducer<S, I, A>(
715
return [hook.memoizedState, dispatch];
716
}
717
718
- // The last update in the entire queue
719
- const last = queue.last;
720
- // The last update that is part of the base state.
721
- const baseUpdate = hook.baseUpdate;
722
- const baseState = hook.baseState;
723
-
724
- // Find the first unprocessed update.
725
- let first;
726
- if (baseUpdate !== null) {
727
- if (last !== null) {
728
- // For the first update, the queue is a circular linked list where
729
- // `queue.last.next = queue.first`. Once the first update commits, and
730
- // the `baseUpdate` is no longer empty, we can unravel the list.
731
- last.next = null;
718
+ const current: Hook = (currentHook: any);
719
+
720
+ // The last rebase update that is NOT part of the base state.
721
+ let baseQueue = current.baseQueue;
722
+
723
+ // The last pending update that hasn't been processed yet.
724
+ let pendingQueue = queue.pending;
725
+ if (pendingQueue !== null) {
726
+ // We have new updates that haven't been processed yet.
727
+ // We'll add them to the base queue.
728
+ if (baseQueue !== null) {
729
+ // Merge the pending queue and the base queue.
730
+ let baseFirst = baseQueue.next;
731
+ let pendingFirst = pendingQueue.next;
732
+ baseQueue.next = pendingFirst;
733
+ pendingQueue.next = baseFirst;
734
}
733
- first = baseUpdate.next;
734
- } else {
735
- first = last !== null ? last.next : null;
735
+ current.baseQueue = baseQueue = pendingQueue;
736
+ queue.pending = null;
737
}
737
- if (first !== null) {
738
- let newState = baseState;
738
+
739
+ if (baseQueue !== null) {
740
+ // We have a queue to process.
741
+ let first = baseQueue.next;
742
+ let newState = current.baseState;
743
+
744
let newBaseState = null;
740
- let newBaseUpdate = null;
741
- let prevUpdate = baseUpdate;
745
+ let newBaseQueueFirst = null;
746
+ let newBaseQueueLast = null;
747
let update = first;
743
- let didSkip = false;
748
do {
749
const updateExpirationTime = update.expirationTime;
750
if (updateExpirationTime < renderExpirationTime) {
751
// Priority is insufficient. Skip this update. If this is the first
752
// skipped update, the previous update/state is the new base
753
// update/state.
750
- if (!didSkip) {
751
- didSkip = true;
752
- newBaseUpdate = prevUpdate;
754
+ const clone: Update<S, A> = {
755
+ expirationTime: update.expirationTime,
756
+ suspenseConfig: update.suspenseConfig,
757
+ action: update.action,
758
+ eagerReducer: update.eagerReducer,
759
+ eagerState: update.eagerState,
760
+ next: (null: any),
761
+ };
762
+ if (newBaseQueueLast === null) {
763
+ newBaseQueueFirst = newBaseQueueLast = clone;
764
newBaseState = newState;
765
+ } else {
766
+ newBaseQueueLast = newBaseQueueLast.next = clone;
767
}
768
// Update the remaining priority in the queue.
769
if (updateExpirationTime > currentlyRenderingFiber.expirationTime) {
@@ -760,6 +773,18 @@ function updateReducer<S, I, A>(
773
} else {
774
// This update does have sufficient priority.
775
776
+ if (newBaseQueueLast !== null) {
777
+ const clone: Update<S, A> = {
778
+ expirationTime: Sync, // This update is going to be committed so we never want uncommit it.
779
+ suspenseConfig: update.suspenseConfig,
780
+ action: update.action,
781
+ eagerReducer: update.eagerReducer,
782
+ eagerState: update.eagerState,
783
+ next: (null: any),
784
+ };
785
+ newBaseQueueLast = newBaseQueueLast.next = clone;
786
+ }
787
+
788
// Mark the event time of this update as relevant to this render pass.
789
// TODO: This should ideally use the true event time of this update rather than
790
// its priority which is a derived and not reverseable value.
@@ -781,13 +806,13 @@ function updateReducer<S, I, A>(
806
newState = reducer(newState, action);
807
}
808
}
784
- prevUpdate = update;
809
update = update.next;
810
} while (update !== null && update !== first);
811
788
- if (!didSkip) {
789
- newBaseUpdate = prevUpdate;
812
+ if (newBaseQueueLast === null) {
813
newBaseState = newState;
814
+ } else {
815
+ newBaseQueueLast.next = (newBaseQueueFirst: any);
816
}
817
818
// Mark that the fiber performed work, but only if the new state is
@@ -797,8 +822,8 @@ function updateReducer<S, I, A>(
822
}
823
824
hook.memoizedState = newState;
800
- hook.baseUpdate = newBaseUpdate;
825
hook.baseState = newBaseState;
826
+ hook.baseQueue = newBaseQueueLast;
827
828
queue.lastRenderedState = newState;
829
}
@@ -816,7 +841,7 @@ function mountState<S>(
841
}
842
hook.memoizedState = hook.baseState = initialState;
843
const queue = (hook.queue = {
819
- last: null,
844
+ pending: null,
845
dispatch: null,
846
lastRenderedReducer: basicStateReducer,
847
lastRenderedState: (initialState: any),
@@ -1233,7 +1258,7 @@ function dispatchAction<S, A>(
1258
action,
1259
eagerReducer: null,
1260
eagerState: null,
1236
- next: null,
1261
+ next: (null: any),
1262
};
1263
if (__DEV__) {
1264
update.priority = getCurrentPriorityLevel();
@@ -1267,7 +1292,7 @@ function dispatchAction<S, A>(
1292
action,
1293
eagerReducer: null,
1294
eagerState: null,
1270
- next: null,
1295
+ next: (null: any),
1296
};
1297
1298
if (__DEV__) {
@@ -1275,19 +1300,15 @@ function dispatchAction<S, A>(
1300
}
1301
1302
// Append the update to the end of the list.
1278
- const last = queue.last;
1279
- if (last === null) {
1303
+ const pending = queue.pending;
1304
+ if (pending === null) {
1305
// This is the first update. Create a circular list.
1306
update.next = update;
1307
} else {
1283
- const first = last.next;
1284
- if (first !== null) {
1285
- // Still circular.
1286
- update.next = first;
1287
- }
1288
- last.next = update;
1308
+ update.next = pending.next;
1309
+ pending.next = update;
1310
}
1290
- queue.last = update;
1311
+ queue.pending = update;
1312
1313
if (
1314
fiber.expirationTime === NoWork &&
packages/react-reconciler/src/ReactFiberWorkLoop.js
+9
-10
@@ -14,6 +14,7 @@ import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
14
import type {Interaction} from 'scheduler/src/Tracing';
15
import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
16
import type {SuspenseState} from './ReactFiberSuspenseComponent';
17
+import type {Hook} from './ReactFiberHooks';
18
19
import {
20
warnAboutDeprecatedLifecycles,
@@ -2859,7 +2860,7 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
2860
// has triggered any high priority updates
2861
const updateQueue = current.updateQueue;
2862
if (updateQueue !== null) {
2862
- let update = updateQueue.firstUpdate;
2863
+ let update = updateQueue.baseQueue;
2864
while (update !== null) {
2865
const priorityLevel = update.priority;
2866
if (
@@ -2883,12 +2884,11 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
2884
break;
2885
case FunctionComponent:
2886
case ForwardRef:
2886
- case SimpleMemoComponent:
2887
- if (
2888
- workInProgressNode.memoizedState !== null &&
2889
- workInProgressNode.memoizedState.baseUpdate !== null
2890
- ) {
2891
- let update = workInProgressNode.memoizedState.baseUpdate;
2887
+ case SimpleMemoComponent: {
2888
+ let firstHook: null | Hook = current.memoizedState;
2889
+ // TODO: This just checks the first Hook. Isn't it suppose to check all Hooks?
2890
+ if (firstHook !== null && firstHook.baseQueue !== null) {
2891
+ let update = firstHook.baseQueue;
2892
// Loop through the functional component's memoized state to see whether
2893
// the component has triggered any high pri updates
2894
while (update !== null) {
@@ -2908,15 +2908,14 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
2908
}
2909
break;
2910
}
2911
- if (
2912
- update.next === workInProgressNode.memoizedState.baseUpdate
2913
- ) {
2911
+ if (update.next === firstHook.baseQueue) {
2912
break;
2913
}
2914
update = update.next;
2915
}
2916
}
2917
break;
2918
+ }
2919
default:
2920
break;
2921
}
packages/react-reconciler/src/ReactUpdateQueue.js
+204
-274
@@ -89,7 +89,7 @@ import type {ExpirationTime} from './ReactFiberExpirationTime';
89
import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
90
import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
91
92
-import {NoWork} from './ReactFiberExpirationTime';
92
+import {NoWork, Sync} from './ReactFiberExpirationTime';
93
import {
94
enterDisallowedContextReadInDEV,
95
exitDisallowedContextReadInDEV,
@@ -117,27 +117,21 @@ export type Update<State> = {
117
payload: any,
118
callback: (() => mixed) | null,
119
120
- next: Update<State> | null,
121
- nextEffect: Update<State> | null,
120
+ next: Update<State>,
121
122
//DEV only
123
priority?: ReactPriorityLevel,
124
};
125
126
+type SharedQueue<State> = {
127
+ pending: Update<State> | null,
128
+};
129
+
130
export type UpdateQueue<State> = {
131
baseState: State,
129
-
130
- firstUpdate: Update<State> | null,
131
- lastUpdate: Update<State> | null,
132
-
133
- firstCapturedUpdate: Update<State> | null,
134
- lastCapturedUpdate: Update<State> | null,
135
-
136
- firstEffect: Update<State> | null,
137
- lastEffect: Update<State> | null,
138
-
139
- firstCapturedEffect: Update<State> | null,
140
- lastCapturedEffect: Update<State> | null,
132
+ baseQueue: Update<State> | null,
133
+ shared: SharedQueue<State>,
134
+ effects: Array<Update<State>> | null,
135
};
136
137
export const UpdateState = 0;
@@ -161,17 +155,14 @@ if (__DEV__) {
155
};
156
}
157
164
-export function createUpdateQueue<State>(baseState: State): UpdateQueue<State> {
158
+function createUpdateQueue<State>(fiber: Fiber): UpdateQueue<State> {
159
const queue: UpdateQueue<State> = {
166
- baseState,
167
- firstUpdate: null,
168
- lastUpdate: null,
169
- firstCapturedUpdate: null,
170
- lastCapturedUpdate: null,
171
- firstEffect: null,
172
- lastEffect: null,
173
- firstCapturedEffect: null,
174
- lastCapturedEffect: null,
160
+ baseState: fiber.memoizedState,
161
+ baseQueue: null,
162
+ shared: {
163
+ pending: null,
164
+ },
165
+ effects: null,
166
};
167
return queue;
168
}
@@ -181,19 +172,9 @@ function cloneUpdateQueue<State>(
172
): UpdateQueue<State> {
173
const queue: UpdateQueue<State> = {
174
baseState: currentQueue.baseState,
184
- firstUpdate: currentQueue.firstUpdate,
185
- lastUpdate: currentQueue.lastUpdate,
186
-
187
- // TODO: With resuming, if we bail out and resuse the child tree, we should
188
- // keep these effects.
189
- firstCapturedUpdate: null,
190
- lastCapturedUpdate: null,
191
-
192
- firstEffect: null,
193
- lastEffect: null,
194
-
195
- firstCapturedEffect: null,
196
- lastCapturedEffect: null,
175
+ baseQueue: currentQueue.baseQueue,
176
+ shared: currentQueue.shared,
177
+ effects: null,
178
};
179
return queue;
180
}
@@ -210,90 +191,46 @@ export function createUpdate(
191
payload: null,
192
callback: null,
193
213
- next: null,
214
- nextEffect: null,
194
+ next: (null: any),
195
};
196
+ update.next = update;
197
if (__DEV__) {
198
update.priority = getCurrentPriorityLevel();
199
}
200
return update;
201
}
202
222
-function appendUpdateToQueue<State>(
223
- queue: UpdateQueue<State>,
224
- update: Update<State>,
225
-) {
226
- // Append the update to the end of the list.
227
- if (queue.lastUpdate === null) {
228
- // Queue is empty
229
- queue.firstUpdate = queue.lastUpdate = update;
230
- } else {
231
- queue.lastUpdate.next = update;
232
- queue.lastUpdate = update;
233
- }
234
-}
235
-
203
export function enqueueUpdate<State>(fiber: Fiber, update: Update<State>) {
237
- // Update queues are created lazily.
238
- const alternate = fiber.alternate;
239
- let queue1;
240
- let queue2;
241
- if (alternate === null) {
242
- // There's only one fiber.
243
- queue1 = fiber.updateQueue;
244
- queue2 = null;
245
- if (queue1 === null) {
246
- queue1 = fiber.updateQueue = createUpdateQueue(fiber.memoizedState);
247
- }
248
- } else {
249
- // There are two owners.
250
- queue1 = fiber.updateQueue;
251
- queue2 = alternate.updateQueue;
252
- if (queue1 === null) {
253
- if (queue2 === null) {
254
- // Neither fiber has an update queue. Create new ones.
255
- queue1 = fiber.updateQueue = createUpdateQueue(fiber.memoizedState);
256
- queue2 = alternate.updateQueue = createUpdateQueue(
257
- alternate.memoizedState,
258
- );
259
- } else {
260
- // Only one fiber has an update queue. Clone to create a new one.
261
- queue1 = fiber.updateQueue = cloneUpdateQueue(queue2);
262
- }
204
+ let sharedQueue;
205
+ let updateQueue = fiber.updateQueue;
206
+ if (updateQueue === null) {
207
+ const alternate = fiber.alternate;
208
+ if (alternate === null) {
209
+ updateQueue = createUpdateQueue(fiber);
210
} else {
264
- if (queue2 === null) {
265
- // Only one fiber has an update queue. Clone to create a new one.
266
- queue2 = alternate.updateQueue = cloneUpdateQueue(queue1);
267
- } else {
268
- // Both owners have an update queue.
211
+ updateQueue = alternate.updateQueue;
212
+ if (updateQueue === null) {
213
+ updateQueue = alternate.updateQueue = createUpdateQueue(alternate);
214
}
215
}
216
+ fiber.updateQueue = updateQueue;
217
}
272
- if (queue2 === null || queue1 === queue2) {
273
- // There's only a single queue.
274
- appendUpdateToQueue(queue1, update);
218
+ sharedQueue = updateQueue.shared;
219
+
220
+ const pending = sharedQueue.pending;
221
+ if (pending === null) {
222
+ // This is the first update. Create a circular list.
223
+ update.next = update;
224
} else {
276
- // There are two queues. We need to append the update to both queues,
277
- // while accounting for the persistent structure of the list — we don't
278
- // want the same update to be added multiple times.
279
- if (queue1.lastUpdate === null || queue2.lastUpdate === null) {
280
- // One of the queues is not empty. We must add the update to both queues.
281
- appendUpdateToQueue(queue1, update);
282
- appendUpdateToQueue(queue2, update);
283
- } else {
284
- // Both queues are non-empty. The last update is the same in both lists,
285
- // because of structural sharing. So, only append to one of the lists.
286
- appendUpdateToQueue(queue1, update);
287
- // But we still need to update the `lastUpdate` pointer of queue2.
288
- queue2.lastUpdate = update;
289
- }
225
+ update.next = pending.next;
226
+ pending.next = update;
227
}
228
+ sharedQueue.pending = update;
229
230
if (__DEV__) {
231
if (
232
fiber.tag === ClassComponent &&
295
- (currentlyProcessingQueue === queue1 ||
296
- (queue2 !== null && currentlyProcessingQueue === queue2)) &&
233
+ currentlyProcessingQueue === sharedQueue &&
234
!didWarnUpdateInsideUpdate
235
) {
236
warningWithoutStack(
@@ -312,12 +249,11 @@ export function enqueueCapturedUpdate<State>(
249
workInProgress: Fiber,
250
update: Update<State>,
251
) {
315
- // Captured updates go into a separate list, and only on the work-in-
316
- // progress queue.
252
+ // Captured updates go only on the work-in-progress queue.
253
let workInProgressQueue = workInProgress.updateQueue;
254
if (workInProgressQueue === null) {
255
workInProgressQueue = workInProgress.updateQueue = createUpdateQueue(
320
- workInProgress.memoizedState,
256
+ workInProgress,
257
);
258
} else {
259
// TODO: I put this here rather than createWorkInProgress so that we don't
@@ -330,12 +266,13 @@ export function enqueueCapturedUpdate<State>(
266
}
267
268
// Append the update to the end of the list.
333
- if (workInProgressQueue.lastCapturedUpdate === null) {
334
- // This is the first render phase update
335
- workInProgressQueue.firstCapturedUpdate = workInProgressQueue.lastCapturedUpdate = update;
269
+ const last = workInProgressQueue.baseQueue;
270
+ if (last === null) {
271
+ workInProgressQueue.baseQueue = update.next = update;
272
+ update.next = update;
273
} else {
337
- workInProgressQueue.lastCapturedUpdate.next = update;
338
- workInProgressQueue.lastCapturedUpdate = update;
274
+ update.next = last.next;
275
+ last.next = update;
276
}
277
}
278
@@ -439,148 +376,162 @@ export function processUpdateQueue<State>(
376
queue = ensureWorkInProgressQueueIsAClone(workInProgress, queue);
377
378
if (__DEV__) {
442
- currentlyProcessingQueue = queue;
379
+ currentlyProcessingQueue = queue.shared;
380
}
381
445
- // These values may change as we process the queue.
446
- let newBaseState = queue.baseState;
447
- let newFirstUpdate = null;
448
- let newExpirationTime = NoWork;
449
-
450
- // Iterate through the list of updates to compute the result.
451
- let update = queue.firstUpdate;
452
- let resultState = newBaseState;
453
- while (update !== null) {
454
- const updateExpirationTime = update.expirationTime;
455
- if (updateExpirationTime < renderExpirationTime) {
456
- // This update does not have sufficient priority. Skip it.
457
- if (newFirstUpdate === null) {
458
- // This is the first skipped update. It will be the first update in
459
- // the new list.
460
- newFirstUpdate = update;
461
- // Since this is the first update that was skipped, the current result
462
- // is the new base state.
463
- newBaseState = resultState;
464
- }
465
- // Since this update will remain in the list, update the remaining
466
- // expiration time.
467
- if (newExpirationTime < updateExpirationTime) {
468
- newExpirationTime = updateExpirationTime;
469
- }
470
- } else {
471
- // This update does have sufficient priority.
472
-
473
- // Mark the event time of this update as relevant to this render pass.
474
- // TODO: This should ideally use the true event time of this update rather than
475
- // its priority which is a derived and not reverseable value.
476
- // TODO: We should skip this update if it was already committed but currently
477
- // we have no way of detecting the difference between a committed and suspended
478
- // update here.
479
- markRenderEventTimeAndConfig(updateExpirationTime, update.suspenseConfig);
480
-
481
- // Process it and compute a new result.
482
- resultState = getStateFromUpdate(
483
- workInProgress,
484
- queue,
485
- update,
486
- resultState,
487
- props,
488
- instance,
489
- );
490
- const callback = update.callback;
491
- if (callback !== null) {
492
- workInProgress.effectTag |= Callback;
493
- // Set this to null, in case it was mutated during an aborted render.
494
- update.nextEffect = null;
495
- if (queue.lastEffect === null) {
496
- queue.firstEffect = queue.lastEffect = update;
497
- } else {
498
- queue.lastEffect.nextEffect = update;
499
- queue.lastEffect = update;
500
- }
382
+ // The last rebase update that is NOT part of the base state.
383
+ let baseQueue = queue.baseQueue;
384
+
385
+ // The last pending update that hasn't been processed yet.
386
+ let pendingQueue = queue.shared.pending;
387
+ if (pendingQueue !== null) {
388
+ // We have new updates that haven't been processed yet.
389
+ // We'll add them to the base queue.
390
+ if (baseQueue !== null) {
391
+ // Merge the pending queue and the base queue.
392
+ let baseFirst = baseQueue.next;
393
+ let pendingFirst = pendingQueue.next;
394
+ baseQueue.next = pendingFirst;
395
+ pendingQueue.next = baseFirst;
396
+ }
397
+
398
+ baseQueue = pendingQueue;
399
+
400
+ queue.shared.pending = null;
401
+ // TODO: Pass `current` as argument
402
+ const current = workInProgress.alternate;
403
+ if (current !== null) {
404
+ const currentQueue = current.updateQueue;
405
+ if (currentQueue !== null) {
406
+ currentQueue.baseQueue = pendingQueue;
407
}
408
}
503
- // Continue to the next update.
504
- update = update.next;
409
}
410
507
- // Separately, iterate though the list of captured updates.
508
- let newFirstCapturedUpdate = null;
509
- update = queue.firstCapturedUpdate;
510
- while (update !== null) {
511
- const updateExpirationTime = update.expirationTime;
512
- if (updateExpirationTime < renderExpirationTime) {
513
- // This update does not have sufficient priority. Skip it.
514
- if (newFirstCapturedUpdate === null) {
515
- // This is the first skipped captured update. It will be the first
516
- // update in the new list.
517
- newFirstCapturedUpdate = update;
518
- // If this is the first update that was skipped, the current result is
519
- // the new base state.
520
- if (newFirstUpdate === null) {
521
- newBaseState = resultState;
522
- }
523
- }
524
- // Since this update will remain in the list, update the remaining
525
- // expiration time.
526
- if (newExpirationTime < updateExpirationTime) {
527
- newExpirationTime = updateExpirationTime;
528
- }
529
- } else {
530
- // This update does have sufficient priority. Process it and compute
531
- // a new result.
532
- resultState = getStateFromUpdate(
533
- workInProgress,
534
- queue,
535
- update,
536
- resultState,
537
- props,
538
- instance,
539
- );
540
- const callback = update.callback;
541
- if (callback !== null) {
542
- workInProgress.effectTag |= Callback;
543
- // Set this to null, in case it was mutated during an aborted render.
544
- update.nextEffect = null;
545
- if (queue.lastCapturedEffect === null) {
546
- queue.firstCapturedEffect = queue.lastCapturedEffect = update;
411
+ // These values may change as we process the queue.
412
+ if (baseQueue !== null) {
413
+ let first = baseQueue.next;
414
+ // Iterate through the list of updates to compute the result.
415
+ let newState = queue.baseState;
416
+ let newExpirationTime = NoWork;
417
+
418
+ let newBaseState = null;
419
+ let newBaseQueueFirst = null;
420
+ let newBaseQueueLast = null;
421
+
422
+ if (first !== null) {
423
+ let update = first;
424
+ do {
425
+ const updateExpirationTime = update.expirationTime;
426
+ if (updateExpirationTime < renderExpirationTime) {
427
+ // Priority is insufficient. Skip this update. If this is the first
428
+ // skipped update, the previous update/state is the new base
429
+ // update/state.
430
+ const clone: Update<State> = {
431
+ expirationTime: update.expirationTime,
432
+ suspenseConfig: update.suspenseConfig,
433
+
434
+ tag: update.tag,
435
+ payload: update.payload,
436
+ callback: update.callback,
437
+
438
+ next: (null: any),
439
+ };
440
+ if (newBaseQueueLast === null) {
441
+ newBaseQueueFirst = newBaseQueueLast = clone;
442
+ newBaseState = newState;
443
+ } else {
444
+ newBaseQueueLast = newBaseQueueLast.next = clone;
445
+ }
446
+ // Update the remaining priority in the queue.
447
+ if (updateExpirationTime > newExpirationTime) {
448
+ newExpirationTime = updateExpirationTime;
449
+ }
450
} else {
548
- queue.lastCapturedEffect.nextEffect = update;
549
- queue.lastCapturedEffect = update;
451
+ // This update does have sufficient priority.
452
+
453
+ if (newBaseQueueLast !== null) {
454
+ const clone: Update<State> = {
455
+ expirationTime: Sync, // This update is going to be committed so we never want uncommit it.
456
+ suspenseConfig: update.suspenseConfig,
457
+
458
+ tag: update.tag,
459
+ payload: update.payload,
460
+ callback: update.callback,
461
+
462
+ next: (null: any),
463
+ };
464
+ newBaseQueueLast = newBaseQueueLast.next = clone;
465
+ }
466
+
467
+ // Mark the event time of this update as relevant to this render pass.
468
+ // TODO: This should ideally use the true event time of this update rather than
469
+ // its priority which is a derived and not reverseable value.
470
+ // TODO: We should skip this update if it was already committed but currently
471
+ // we have no way of detecting the difference between a committed and suspended
472
+ // update here.
473
+ markRenderEventTimeAndConfig(
474
+ updateExpirationTime,
475
+ update.suspenseConfig,
476
+ );
477
+
478
+ // Process this update.
479
+ newState = getStateFromUpdate(
480
+ workInProgress,
481
+ queue,
482
+ update,
483
+ newState,
484
+ props,
485
+ instance,
486
+ );
487
+ const callback = update.callback;
488
+ if (callback !== null) {
489
+ workInProgress.effectTag |= Callback;
490
+ let effects = queue.effects;
491
+ if (effects === null) {
492
+ queue.effects = [update];
493
+ } else {
494
+ effects.push(update);
495
+ }
496
+ }
497
}
551
- }
498
+ update = update.next;
499
+ if (update === null || update === first) {
500
+ pendingQueue = queue.shared.pending;
501
+ if (pendingQueue === null) {
502
+ break;
503
+ } else {
504
+ // An update was scheduled from inside a reducer. Add the new
505
+ // pending updates to the end of the list and keep processing.
506
+ update = baseQueue.next = pendingQueue.next;
507
+ pendingQueue.next = first;
508
+ queue.baseQueue = baseQueue = pendingQueue;
509
+ queue.shared.pending = null;
510
+ }
511
+ }
512
+ } while (true);
513
}
553
- update = update.next;
554
- }
514
556
- if (newFirstUpdate === null) {
557
- queue.lastUpdate = null;
558
- }
559
- if (newFirstCapturedUpdate === null) {
560
- queue.lastCapturedUpdate = null;
561
- } else {
562
- workInProgress.effectTag |= Callback;
563
- }
564
- if (newFirstUpdate === null && newFirstCapturedUpdate === null) {
565
- // We processed every update, without skipping. That means the new base
566
- // state is the same as the result state.
567
- newBaseState = resultState;
568
- }
515
+ if (newBaseQueueLast === null) {
516
+ newBaseState = newState;
517
+ } else {
518
+ newBaseQueueLast.next = (newBaseQueueFirst: any);
519
+ }
520
570
- queue.baseState = newBaseState;
571
- queue.firstUpdate = newFirstUpdate;
572
- queue.firstCapturedUpdate = newFirstCapturedUpdate;
573
-
574
- // Set the remaining expiration time to be whatever is remaining in the queue.
575
- // This should be fine because the only two other things that contribute to
576
- // expiration time are props and context. We're already in the middle of the
577
- // begin phase by the time we start processing the queue, so we've already
578
- // dealt with the props. Context in components that specify
579
- // shouldComponentUpdate is tricky; but we'll have to account for
580
- // that regardless.
581
- markUnprocessedUpdateTime(newExpirationTime);
582
- workInProgress.expirationTime = newExpirationTime;
583
- workInProgress.memoizedState = resultState;
521
+ queue.baseState = ((newBaseState: any): State);
522
+ queue.baseQueue = newBaseQueueLast;
523
+
524
+ // Set the remaining expiration time to be whatever is remaining in the queue.
525
+ // This should be fine because the only two other things that contribute to
526
+ // expiration time are props and context. We're already in the middle of the
527
+ // begin phase by the time we start processing the queue, so we've already
528
+ // dealt with the props. Context in components that specify
529
+ // shouldComponentUpdate is tricky; but we'll have to account for
530
+ // that regardless.
531
+ markUnprocessedUpdateTime(newExpirationTime);
532
+ workInProgress.expirationTime = newExpirationTime;
533
+ workInProgress.memoizedState = newState;
534
+ }
535
536
if (__DEV__) {
537
currentlyProcessingQueue = null;
@@ -611,38 +562,17 @@ export function commitUpdateQueue<State>(
562
instance: any,
563
renderExpirationTime: ExpirationTime,
564
): void {
614
- // If the finished render included captured updates, and there are still
615
- // lower priority updates left over, we need to keep the captured updates
616
- // in the queue so that they are rebased and not dropped once we process the
617
- // queue again at the lower priority.
618
- if (finishedQueue.firstCapturedUpdate !== null) {
619
- // Join the captured update list to the end of the normal list.
620
- if (finishedQueue.lastUpdate !== null) {
621
- finishedQueue.lastUpdate.next = finishedQueue.firstCapturedUpdate;
622
- finishedQueue.lastUpdate = finishedQueue.lastCapturedUpdate;
623
- }
624
- // Clear the list of captured updates.
625
- finishedQueue.firstCapturedUpdate = finishedQueue.lastCapturedUpdate = null;
626
- }
627
-
565
// Commit the effects
629
- commitUpdateEffects(finishedQueue.firstEffect, instance);
630
- finishedQueue.firstEffect = finishedQueue.lastEffect = null;
631
-
632
- commitUpdateEffects(finishedQueue.firstCapturedEffect, instance);
633
- finishedQueue.firstCapturedEffect = finishedQueue.lastCapturedEffect = null;
634
-}
635
-
636
-function commitUpdateEffects<State>(
637
- effect: Update<State> | null,
638
- instance: any,
639
-): void {
640
- while (effect !== null) {
641
- const callback = effect.callback;
642
- if (callback !== null) {
643
- effect.callback = null;
644
- callCallback(callback, instance);
566
+ const effects = finishedQueue.effects;
567
+ finishedQueue.effects = null;
568
+ if (effects !== null) {
569
+ for (let i = 0; i < effects.length; i++) {
570
+ const effect = effects[i];
571
+ const callback = effect.callback;
572
+ if (callback !== null) {
573
+ effect.callback = null;
574
+ callCallback(callback, instance);
575
+ }
576
}
646
- effect = effect.nextEffect;
577
}
578
}
packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js
+172
@@ -653,4 +653,176 @@ describe('ReactIncrementalUpdates', () => {
653
expect(Scheduler).toFlushAndYield(['Commit: goodbye']);
654
});
655
});
656
+
657
+ it('when rebasing, does not exclude updates that were already committed, regardless of priority', async () => {
658
+ const {useState, useLayoutEffect} = React;
659
+
660
+ let pushToLog;
661
+ function App() {
662
+ const [log, setLog] = useState('');
663
+ pushToLog = msg => {
664
+ setLog(prevLog => prevLog + msg);
665
+ };
666
+
667
+ useLayoutEffect(
668
+ () => {
669
+ Scheduler.unstable_yieldValue('Committed: ' + log);
670
+ if (log === 'B') {
671
+ // Right after B commits, schedule additional updates.
672
+ Scheduler.unstable_runWithPriority(
673
+ Scheduler.unstable_UserBlockingPriority,
674
+ () => {
675
+ pushToLog('C');
676
+ },
677
+ );
678
+ setLog(prevLog => prevLog + 'D');
679
+ }
680
+ },
681
+ [log],
682
+ );
683
+
684
+ return log;
685
+ }
686
+
687
+ const root = ReactNoop.createRoot();
688
+ await ReactNoop.act(async () => {
689
+ root.render(<App />);
690
+ });
691
+ expect(Scheduler).toHaveYielded(['Committed: ']);
692
+ expect(root).toMatchRenderedOutput('');
693
+
694
+ await ReactNoop.act(async () => {
695
+ pushToLog('A');
696
+ Scheduler.unstable_runWithPriority(
697
+ Scheduler.unstable_UserBlockingPriority,
698
+ () => {
699
+ pushToLog('B');
700
+ },
701
+ );
702
+ });
703
+ expect(Scheduler).toHaveYielded([
704
+ // A and B are pending. B is higher priority, so we'll render that first.
705
+ 'Committed: B',
706
+ // Because A comes first in the queue, we're now in rebase mode. B must
707
+ // be rebased on top of A. Also, in a layout effect, we received two new
708
+ // updates: C and D. C is user-blocking and D is synchronous.
709
+ //
710
+ // First render the synchronous update. What we're testing here is that
711
+ // B *is not dropped* even though it has lower than sync priority. That's
712
+ // because we already committed it. However, this render should not
713
+ // include C, because that update wasn't already committed.
714
+ 'Committed: BD',
715
+ 'Committed: BCD',
716
+ 'Committed: ABCD',
717
+ ]);
718
+ expect(root).toMatchRenderedOutput('ABCD');
719
+ });
720
+
721
+ it('when rebasing, does not exclude updates that were already committed, regardless of priority (classes)', async () => {
722
+ let pushToLog;
723
+ class App extends React.Component {
724
+ state = {log: ''};
725
+ pushToLog = msg => {
726
+ this.setState(prevState => ({log: prevState.log + msg}));
727
+ };
728
+ componentDidUpdate() {
729
+ Scheduler.unstable_yieldValue('Committed: ' + this.state.log);
730
+ if (this.state.log === 'B') {
731
+ // Right after B commits, schedule additional updates.
732
+ Scheduler.unstable_runWithPriority(
733
+ Scheduler.unstable_UserBlockingPriority,
734
+ () => {
735
+ this.pushToLog('C');
736
+ },
737
+ );
738
+ this.pushToLog('D');
739
+ }
740
+ }
741
+ render() {
742
+ pushToLog = this.pushToLog;
743
+ return this.state.log;
744
+ }
745
+ }
746
+
747
+ const root = ReactNoop.createRoot();
748
+ await ReactNoop.act(async () => {
749
+ root.render(<App />);
750
+ });
751
+ expect(Scheduler).toHaveYielded([]);
752
+ expect(root).toMatchRenderedOutput('');
753
+
754
+ await ReactNoop.act(async () => {
755
+ pushToLog('A');
756
+ Scheduler.unstable_runWithPriority(
757
+ Scheduler.unstable_UserBlockingPriority,
758
+ () => {
759
+ pushToLog('B');
760
+ },
761
+ );
762
+ });
763
+ expect(Scheduler).toHaveYielded([
764
+ // A and B are pending. B is higher priority, so we'll render that first.
765
+ 'Committed: B',
766
+ // Because A comes first in the queue, we're now in rebase mode. B must
767
+ // be rebased on top of A. Also, in a layout effect, we received two new
768
+ // updates: C and D. C is user-blocking and D is synchronous.
769
+ //
770
+ // First render the synchronous update. What we're testing here is that
771
+ // B *is not dropped* even though it has lower than sync priority. That's
772
+ // because we already committed it. However, this render should not
773
+ // include C, because that update wasn't already committed.
774
+ 'Committed: BD',
775
+ 'Committed: BCD',
776
+ 'Committed: ABCD',
777
+ ]);
778
+ expect(root).toMatchRenderedOutput('ABCD');
779
+ });
780
+
781
+ it("base state of update queue is initialized to its fiber's memoized state", async () => {
782
+ // This test is very weird because it tests an implementation detail but
783
+ // is tested in terms of public APIs. When it was originally written, the
784
+ // test failed because the update queue was initialized to the state of
785
+ // the alternate fiber.
786
+ let app;
787
+ class App extends React.Component {
788
+ state = {prevProp: 'A', count: 0};
789
+ static getDerivedStateFromProps(props, state) {
790
+ // Add 100 whenever the label prop changes. The prev label is stored
791
+ // in state. If the state is dropped incorrectly, we'll fail to detect
792
+ // prop changes.
793
+ if (props.prop !== state.prevProp) {
794
+ return {
795
+ prevProp: props.prop,
796
+ count: state.count + 100,
797
+ };
798
+ }
799
+ return null;
800
+ }
801
+ render() {
802
+ app = this;
803
+ return this.state.count;
804
+ }
805
+ }
806
+
807
+ const root = ReactNoop.createRoot();
808
+ await ReactNoop.act(async () => {
809
+ root.render(<App prop="A" />);
810
+ });
811
+ expect(root).toMatchRenderedOutput('0');
812
+
813
+ // Changing the prop causes the count to increase by 100
814
+ await ReactNoop.act(async () => {
815
+ root.render(<App prop="B" />);
816
+ });
817
+ expect(root).toMatchRenderedOutput('100');
818
+
819
+ // Now increment the count by 1 with a state update. And, in the same
820
+ // batch, change the prop back to its original value.
821
+ await ReactNoop.act(async () => {
822
+ root.render(<App prop="A" />);
823
+ app.setState(state => ({count: state.count + 1}));
824
+ });
825
+ // There were two total prop changes, plus an increment.
826
+ expect(root).toMatchRenderedOutput('201');
827
+ });
828
});