@samitouri / QOS-React / commits / e039e690b5

Revert Update Queue Refactor

Reverts b617db3d966f678eb0b4aac6d96f7967b37a9e91. Found some bugs when attempting to land in www. Reverting to fix master. I'll land again *after* the change successfully land downstream.

Andrew Clark committed Dec 9, 2019 at 13:19 UTC e039e690b5c45c458dd4026f3db16bac18ed0e47
5 files changed +344 -479
packages/react-noop-renderer/src/createReactNoop.js
+11 -24
@@ -1142,33 +1142,20 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
1142
1143 function logUpdateQueue(updateQueue: UpdateQueue<mixed>, depth) {
1144 log(' '.repeat(depth + 1) + 'QUEUED UPDATES');
1145 - const last = updateQueue.baseQueue;
1146 - if (last === null) {
1145 + const firstUpdate = updateQueue.firstUpdate;
1146 + if (!firstUpdate) {
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 - }
1149
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 - }
1150 + log(
1151 + ' '.repeat(depth + 1) + '~',
1152 + '[' + firstUpdate.expirationTime + ']',
1153 + );
1154 + while (firstUpdate.next) {
1155 + log(
1156 + ' '.repeat(depth + 1) + '~',
1157 + '[' + firstUpdate.expirationTime + ']',
1158 + );
1159 }
1160 }
1161
packages/react-reconciler/src/ReactFiberHooks.js
+49 -70
@@ -20,7 +20,7 @@ import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
20
21 import ReactSharedInternals from 'shared/ReactSharedInternals';
22
23 -import {NoWork, Sync} from './ReactFiberExpirationTime';
23 +import {NoWork} 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>,
111 + next: Update<S, A> | null,
112
113 priority?: ReactPriorityLevel,
114 };
115
116 type UpdateQueue<S, A> = {
117 - pending: Update<S, A> | null,
117 + last: 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 - baseQueue: Update<any, any> | null,
147 + baseUpdate: 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,
547 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,
607 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 - pending: null,
648 + last: 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.baseQueue === null) {
706 + if (hook.baseUpdate === queue.last) {
707 hook.baseState = newState;
708 }
709
@@ -715,55 +715,42 @@ function updateReducer<S, I, A>(
715 return [hook.memoizedState, dispatch];
716 }
717
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;
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;
732 }
735 - current.baseQueue = baseQueue = pendingQueue;
736 - queue.pending = null;
733 + first = baseUpdate.next;
734 + } else {
735 + first = last !== null ? last.next : null;
736 }
738 -
739 - if (baseQueue !== null) {
740 - // We have a queue to process.
741 - let first = baseQueue.next;
742 - let newState = current.baseState;
743 -
737 + if (first !== null) {
738 + let newState = baseState;
739 let newBaseState = null;
745 - let newBaseQueueFirst = null;
746 - let newBaseQueueLast = null;
740 + let newBaseUpdate = null;
741 + let prevUpdate = baseUpdate;
742 let update = first;
743 + let didSkip = false;
744 do {
745 const updateExpirationTime = update.expirationTime;
746 if (updateExpirationTime < renderExpirationTime) {
747 // Priority is insufficient. Skip this update. If this is the first
748 // skipped update, the previous update/state is the new base
749 // update/state.
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;
750 + if (!didSkip) {
751 + didSkip = true;
752 + newBaseUpdate = prevUpdate;
753 newBaseState = newState;
765 - } else {
766 - newBaseQueueLast = newBaseQueueLast.next = clone;
754 }
755 // Update the remaining priority in the queue.
756 if (updateExpirationTime > currentlyRenderingFiber.expirationTime) {
@@ -773,18 +760,6 @@ function updateReducer<S, I, A>(
760 } else {
761 // This update does have sufficient priority.
762
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 -
763 // Mark the event time of this update as relevant to this render pass.
764 // TODO: This should ideally use the true event time of this update rather than
765 // its priority which is a derived and not reverseable value.
@@ -806,13 +781,13 @@ function updateReducer<S, I, A>(
781 newState = reducer(newState, action);
782 }
783 }
784 + prevUpdate = update;
785 update = update.next;
786 } while (update !== null && update !== first);
787
812 - if (newBaseQueueLast === null) {
788 + if (!didSkip) {
789 + newBaseUpdate = prevUpdate;
790 newBaseState = newState;
814 - } else {
815 - newBaseQueueLast.next = (newBaseQueueFirst: any);
791 }
792
793 // Mark that the fiber performed work, but only if the new state is
@@ -822,8 +797,8 @@ function updateReducer<S, I, A>(
797 }
798
799 hook.memoizedState = newState;
800 + hook.baseUpdate = newBaseUpdate;
801 hook.baseState = newBaseState;
826 - hook.baseQueue = newBaseQueueLast;
802
803 queue.lastRenderedState = newState;
804 }
@@ -841,7 +816,7 @@ function mountState<S>(
816 }
817 hook.memoizedState = hook.baseState = initialState;
818 const queue = (hook.queue = {
844 - pending: null,
819 + last: null,
820 dispatch: null,
821 lastRenderedReducer: basicStateReducer,
822 lastRenderedState: (initialState: any),
@@ -1258,7 +1233,7 @@ function dispatchAction<S, A>(
1233 action,
1234 eagerReducer: null,
1235 eagerState: null,
1261 - next: (null: any),
1236 + next: null,
1237 };
1238 if (__DEV__) {
1239 update.priority = getCurrentPriorityLevel();
@@ -1292,7 +1267,7 @@ function dispatchAction<S, A>(
1267 action,
1268 eagerReducer: null,
1269 eagerState: null,
1295 - next: (null: any),
1270 + next: null,
1271 };
1272
1273 if (__DEV__) {
@@ -1300,15 +1275,19 @@ function dispatchAction<S, A>(
1275 }
1276
1277 // Append the update to the end of the list.
1303 - const pending = queue.pending;
1304 - if (pending === null) {
1278 + const last = queue.last;
1279 + if (last === null) {
1280 // This is the first update. Create a circular list.
1281 update.next = update;
1282 } else {
1308 - update.next = pending.next;
1309 - pending.next = update;
1283 + const first = last.next;
1284 + if (first !== null) {
1285 + // Still circular.
1286 + update.next = first;
1287 + }
1288 + last.next = update;
1289 }
1311 - queue.pending = update;
1290 + queue.last = update;
1291
1292 if (
1293 fiber.expirationTime === NoWork &&
packages/react-reconciler/src/ReactFiberWorkLoop.js
+10 -9
@@ -14,7 +14,6 @@ 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';
17
18 import {
19 warnAboutDeprecatedLifecycles,
@@ -2860,7 +2859,7 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
2859 // has triggered any high priority updates
2860 const updateQueue = current.updateQueue;
2861 if (updateQueue !== null) {
2863 - let update = updateQueue.baseQueue;
2862 + let update = updateQueue.firstUpdate;
2863 while (update !== null) {
2864 const priorityLevel = update.priority;
2865 if (
@@ -2884,11 +2883,12 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
2883 break;
2884 case FunctionComponent:
2885 case ForwardRef:
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;
2886 + case SimpleMemoComponent:
2887 + if (
2888 + workInProgressNode.memoizedState !== null &&
2889 + workInProgressNode.memoizedState.baseUpdate !== null
2890 + ) {
2891 + let update = workInProgressNode.memoizedState.baseUpdate;
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,14 +2908,15 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
2908 }
2909 break;
2910 }
2911 - if (update.next === firstHook.baseQueue) {
2911 + if (
2912 + update.next === workInProgressNode.memoizedState.baseUpdate
2913 + ) {
2914 break;
2915 }
2916 update = update.next;
2917 }
2918 }
2919 break;
2918 - }
2920 default:
2921 break;
2922 }
packages/react-reconciler/src/ReactUpdateQueue.js
+274 -204
@@ -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, Sync} from './ReactFiberExpirationTime';
92 +import {NoWork} from './ReactFiberExpirationTime';
93 import {
94 enterDisallowedContextReadInDEV,
95 exitDisallowedContextReadInDEV,
@@ -117,21 +117,27 @@ export type Update<State> = {
117 payload: any,
118 callback: (() => mixed) | null,
119
120 - next: Update<State>,
120 + next: Update<State> | null,
121 + nextEffect: Update<State> | null,
122
123 //DEV only
124 priority?: ReactPriorityLevel,
125 };
126
126 -type SharedQueue<State> = {
127 - pending: Update<State> | null,
128 -};
129 -
127 export type UpdateQueue<State> = {
128 baseState: State,
132 - baseQueue: Update<State> | null,
133 - shared: SharedQueue<State>,
134 - effects: Array<Update<State>> | null,
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,
141 };
142
143 export const UpdateState = 0;
@@ -155,14 +161,17 @@ if (__DEV__) {
161 };
162 }
163
158 -function createUpdateQueue<State>(fiber: Fiber): UpdateQueue<State> {
164 +export function createUpdateQueue<State>(baseState: State): UpdateQueue<State> {
165 const queue: UpdateQueue<State> = {
160 - baseState: fiber.memoizedState,
161 - baseQueue: null,
162 - shared: {
163 - pending: null,
164 - },
165 - effects: null,
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,
175 };
176 return queue;
177 }
@@ -172,9 +181,19 @@ function cloneUpdateQueue<State>(
181 ): UpdateQueue<State> {
182 const queue: UpdateQueue<State> = {
183 baseState: currentQueue.baseState,
175 - baseQueue: currentQueue.baseQueue,
176 - shared: currentQueue.shared,
177 - effects: null,
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,
197 };
198 return queue;
199 }
@@ -191,46 +210,90 @@ export function createUpdate(
210 payload: null,
211 callback: null,
212
194 - next: (null: any),
213 + next: null,
214 + nextEffect: null,
215 };
196 - update.next = update;
216 if (__DEV__) {
217 update.priority = getCurrentPriorityLevel();
218 }
219 return update;
220 }
221
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 +
236 export function enqueueUpdate<State>(fiber: Fiber, update: Update<State>) {
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);
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 + }
263 } else {
211 - updateQueue = alternate.updateQueue;
212 - if (updateQueue === null) {
213 - updateQueue = alternate.updateQueue = createUpdateQueue(alternate);
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.
269 }
270 }
216 - fiber.updateQueue = updateQueue;
271 }
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;
272 + if (queue2 === null || queue1 === queue2) {
273 + // There's only a single queue.
274 + appendUpdateToQueue(queue1, update);
275 } else {
225 - update.next = pending.next;
226 - pending.next = update;
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 + }
290 }
228 - sharedQueue.pending = update;
291
292 if (__DEV__) {
293 if (
294 fiber.tag === ClassComponent &&
233 - currentlyProcessingQueue === sharedQueue &&
295 + (currentlyProcessingQueue === queue1 ||
296 + (queue2 !== null && currentlyProcessingQueue === queue2)) &&
297 !didWarnUpdateInsideUpdate
298 ) {
299 warningWithoutStack(
@@ -249,11 +312,12 @@ export function enqueueCapturedUpdate<State>(
312 workInProgress: Fiber,
313 update: Update<State>,
314 ) {
252 - // Captured updates go only on the work-in-progress queue.
315 + // Captured updates go into a separate list, and only on the work-in-
316 + // progress queue.
317 let workInProgressQueue = workInProgress.updateQueue;
318 if (workInProgressQueue === null) {
319 workInProgressQueue = workInProgress.updateQueue = createUpdateQueue(
256 - workInProgress,
320 + workInProgress.memoizedState,
321 );
322 } else {
323 // TODO: I put this here rather than createWorkInProgress so that we don't
@@ -266,13 +330,12 @@ export function enqueueCapturedUpdate<State>(
330 }
331
332 // Append the update to the end of the list.
269 - const last = workInProgressQueue.baseQueue;
270 - if (last === null) {
271 - workInProgressQueue.baseQueue = update.next = update;
272 - update.next = update;
333 + if (workInProgressQueue.lastCapturedUpdate === null) {
334 + // This is the first render phase update
335 + workInProgressQueue.firstCapturedUpdate = workInProgressQueue.lastCapturedUpdate = update;
336 } else {
274 - update.next = last.next;
275 - last.next = update;
337 + workInProgressQueue.lastCapturedUpdate.next = update;
338 + workInProgressQueue.lastCapturedUpdate = update;
339 }
340 }
341
@@ -376,163 +439,149 @@ export function processUpdateQueue<State>(
439 queue = ensureWorkInProgressQueueIsAClone(workInProgress, queue);
440
441 if (__DEV__) {
379 - currentlyProcessingQueue = queue.shared;
380 - }
381 -
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 - }
442 + currentlyProcessingQueue = queue;
443 }
444
445 // 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 - }
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 {
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 - }
498 + queue.lastEffect.nextEffect = update;
499 + queue.lastEffect = update;
500 }
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);
501 + }
502 }
503 + // Continue to the next update.
504 + update = update.next;
505 + }
506
515 - if (newBaseQueueLast === null) {
516 - newBaseState = newState;
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 {
518 - newBaseQueueLast.next = (newBaseQueueFirst: any);
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;
547 + } else {
548 + queue.lastCapturedEffect.nextEffect = update;
549 + queue.lastCapturedEffect = update;
550 + }
551 + }
552 }
553 + update = update.next;
554 + }
555
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;
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 }
569
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;
584 +
585 if (__DEV__) {
586 currentlyProcessingQueue = null;
587 }
@@ -562,17 +611,38 @@ export function commitUpdateQueue<State>(
611 instance: any,
612 renderExpirationTime: ExpirationTime,
613 ): 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 +
628 // Commit the effects
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 - }
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);
645 }
646 + effect = effect.nextEffect;
647 }
648 }
packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js
-172
@@ -653,176 +653,4 @@ 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 - });
656 });