diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 61a14a94af4d..24a368056c0a 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -1142,33 +1142,20 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { function logUpdateQueue(updateQueue: UpdateQueue, depth) { log(' '.repeat(depth + 1) + 'QUEUED UPDATES'); - const last = updateQueue.baseQueue; - if (last === null) { + const firstUpdate = updateQueue.firstUpdate; + if (!firstUpdate) { return; } - const first = last.next; - let update = first; - if (update !== null) { - do { - log( - ' '.repeat(depth + 1) + '~', - '[' + update.expirationTime + ']', - ); - } while (update !== null && update !== first); - } - const lastPending = updateQueue.shared.pending; - if (lastPending !== null) { - const firstPending = lastPending.next; - let pendingUpdate = firstPending; - if (pendingUpdate !== null) { - do { - log( - ' '.repeat(depth + 1) + '~', - '[' + pendingUpdate.expirationTime + ']', - ); - } while (pendingUpdate !== null && pendingUpdate !== firstPending); - } + log( + ' '.repeat(depth + 1) + '~', + '[' + firstUpdate.expirationTime + ']', + ); + while (firstUpdate.next) { + log( + ' '.repeat(depth + 1) + '~', + '[' + firstUpdate.expirationTime + ']', + ); } } diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 0a6c3a132cc2..7a4c7dfae82b 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -20,7 +20,7 @@ import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; import ReactSharedInternals from 'shared/ReactSharedInternals'; -import {NoWork, Sync} from './ReactFiberExpirationTime'; +import {NoWork} from './ReactFiberExpirationTime'; import {readContext} from './ReactFiberNewContext'; import {createResponderListener} from './ReactFiberEvents'; import { @@ -108,13 +108,13 @@ type Update = { action: A, eagerReducer: ((S, A) => S) | null, eagerState: S | null, - next: Update, + next: Update | null, priority?: ReactPriorityLevel, }; type UpdateQueue = { - pending: Update | null, + last: Update | null, dispatch: (A => mixed) | null, lastRenderedReducer: ((S, A) => S) | null, lastRenderedState: S | null, @@ -144,7 +144,7 @@ export type Hook = { memoizedState: any, baseState: any, - baseQueue: Update | null, + baseUpdate: Update | null, queue: UpdateQueue | null, next: Hook | null, @@ -544,8 +544,8 @@ function mountWorkInProgressHook(): Hook { memoizedState: null, baseState: null, - baseQueue: null, queue: null, + baseUpdate: null, next: null, }; @@ -604,8 +604,8 @@ function updateWorkInProgressHook(): Hook { memoizedState: currentHook.memoizedState, baseState: currentHook.baseState, - baseQueue: currentHook.baseQueue, queue: currentHook.queue, + baseUpdate: currentHook.baseUpdate, next: null, }; @@ -645,7 +645,7 @@ function mountReducer( } hook.memoizedState = hook.baseState = initialState; const queue = (hook.queue = { - pending: null, + last: null, dispatch: null, lastRenderedReducer: reducer, lastRenderedState: (initialState: any), @@ -703,7 +703,7 @@ function updateReducer( // the base state unless the queue is empty. // TODO: Not sure if this is the desired semantics, but it's what we // do for gDSFP. I can't remember why. - if (hook.baseQueue === null) { + if (hook.baseUpdate === queue.last) { hook.baseState = newState; } @@ -715,55 +715,42 @@ function updateReducer( return [hook.memoizedState, dispatch]; } - const current: Hook = (currentHook: any); - - // The last rebase update that is NOT part of the base state. - let baseQueue = current.baseQueue; - - // The last pending update that hasn't been processed yet. - let pendingQueue = queue.pending; - if (pendingQueue !== null) { - // We have new updates that haven't been processed yet. - // We'll add them to the base queue. - if (baseQueue !== null) { - // Merge the pending queue and the base queue. - let baseFirst = baseQueue.next; - let pendingFirst = pendingQueue.next; - baseQueue.next = pendingFirst; - pendingQueue.next = baseFirst; + // The last update in the entire queue + const last = queue.last; + // The last update that is part of the base state. + const baseUpdate = hook.baseUpdate; + const baseState = hook.baseState; + + // Find the first unprocessed update. + let first; + if (baseUpdate !== null) { + if (last !== null) { + // For the first update, the queue is a circular linked list where + // `queue.last.next = queue.first`. Once the first update commits, and + // the `baseUpdate` is no longer empty, we can unravel the list. + last.next = null; } - current.baseQueue = baseQueue = pendingQueue; - queue.pending = null; + first = baseUpdate.next; + } else { + first = last !== null ? last.next : null; } - - if (baseQueue !== null) { - // We have a queue to process. - let first = baseQueue.next; - let newState = current.baseState; - + if (first !== null) { + let newState = baseState; let newBaseState = null; - let newBaseQueueFirst = null; - let newBaseQueueLast = null; + let newBaseUpdate = null; + let prevUpdate = baseUpdate; let update = first; + let didSkip = false; do { const updateExpirationTime = update.expirationTime; if (updateExpirationTime < renderExpirationTime) { // Priority is insufficient. Skip this update. If this is the first // skipped update, the previous update/state is the new base // update/state. - const clone: Update = { - expirationTime: update.expirationTime, - suspenseConfig: update.suspenseConfig, - action: update.action, - eagerReducer: update.eagerReducer, - eagerState: update.eagerState, - next: (null: any), - }; - if (newBaseQueueLast === null) { - newBaseQueueFirst = newBaseQueueLast = clone; + if (!didSkip) { + didSkip = true; + newBaseUpdate = prevUpdate; newBaseState = newState; - } else { - newBaseQueueLast = newBaseQueueLast.next = clone; } // Update the remaining priority in the queue. if (updateExpirationTime > currentlyRenderingFiber.expirationTime) { @@ -773,18 +760,6 @@ function updateReducer( } else { // This update does have sufficient priority. - if (newBaseQueueLast !== null) { - const clone: Update = { - expirationTime: Sync, // This update is going to be committed so we never want uncommit it. - suspenseConfig: update.suspenseConfig, - action: update.action, - eagerReducer: update.eagerReducer, - eagerState: update.eagerState, - next: (null: any), - }; - newBaseQueueLast = newBaseQueueLast.next = clone; - } - // Mark the event time of this update as relevant to this render pass. // TODO: This should ideally use the true event time of this update rather than // its priority which is a derived and not reverseable value. @@ -806,13 +781,13 @@ function updateReducer( newState = reducer(newState, action); } } + prevUpdate = update; update = update.next; } while (update !== null && update !== first); - if (newBaseQueueLast === null) { + if (!didSkip) { + newBaseUpdate = prevUpdate; newBaseState = newState; - } else { - newBaseQueueLast.next = (newBaseQueueFirst: any); } // Mark that the fiber performed work, but only if the new state is @@ -822,8 +797,8 @@ function updateReducer( } hook.memoizedState = newState; + hook.baseUpdate = newBaseUpdate; hook.baseState = newBaseState; - hook.baseQueue = newBaseQueueLast; queue.lastRenderedState = newState; } @@ -841,7 +816,7 @@ function mountState( } hook.memoizedState = hook.baseState = initialState; const queue = (hook.queue = { - pending: null, + last: null, dispatch: null, lastRenderedReducer: basicStateReducer, lastRenderedState: (initialState: any), @@ -1258,7 +1233,7 @@ function dispatchAction( action, eagerReducer: null, eagerState: null, - next: (null: any), + next: null, }; if (__DEV__) { update.priority = getCurrentPriorityLevel(); @@ -1292,7 +1267,7 @@ function dispatchAction( action, eagerReducer: null, eagerState: null, - next: (null: any), + next: null, }; if (__DEV__) { @@ -1300,15 +1275,19 @@ function dispatchAction( } // Append the update to the end of the list. - const pending = queue.pending; - if (pending === null) { + const last = queue.last; + if (last === null) { // This is the first update. Create a circular list. update.next = update; } else { - update.next = pending.next; - pending.next = update; + const first = last.next; + if (first !== null) { + // Still circular. + update.next = first; + } + last.next = update; } - queue.pending = update; + queue.last = update; if ( fiber.expirationTime === NoWork && diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 649f37a410ab..8273d92b3c7a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -14,7 +14,6 @@ import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; import type {Interaction} from 'scheduler/src/Tracing'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import type {SuspenseState} from './ReactFiberSuspenseComponent'; -import type {Hook} from './ReactFiberHooks'; import { warnAboutDeprecatedLifecycles, @@ -2860,7 +2859,7 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) { // has triggered any high priority updates const updateQueue = current.updateQueue; if (updateQueue !== null) { - let update = updateQueue.baseQueue; + let update = updateQueue.firstUpdate; while (update !== null) { const priorityLevel = update.priority; if ( @@ -2884,11 +2883,12 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) { break; case FunctionComponent: case ForwardRef: - case SimpleMemoComponent: { - let firstHook: null | Hook = current.memoizedState; - // TODO: This just checks the first Hook. Isn't it suppose to check all Hooks? - if (firstHook !== null && firstHook.baseQueue !== null) { - let update = firstHook.baseQueue; + case SimpleMemoComponent: + if ( + workInProgressNode.memoizedState !== null && + workInProgressNode.memoizedState.baseUpdate !== null + ) { + let update = workInProgressNode.memoizedState.baseUpdate; // Loop through the functional component's memoized state to see whether // the component has triggered any high pri updates while (update !== null) { @@ -2908,14 +2908,15 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) { } break; } - if (update.next === firstHook.baseQueue) { + if ( + update.next === workInProgressNode.memoizedState.baseUpdate + ) { break; } update = update.next; } } break; - } default: break; } diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index 10c7e55274e8..0d2a8f68230b 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -89,7 +89,7 @@ import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; -import {NoWork, Sync} from './ReactFiberExpirationTime'; +import {NoWork} from './ReactFiberExpirationTime'; import { enterDisallowedContextReadInDEV, exitDisallowedContextReadInDEV, @@ -117,21 +117,27 @@ export type Update = { payload: any, callback: (() => mixed) | null, - next: Update, + next: Update | null, + nextEffect: Update | null, //DEV only priority?: ReactPriorityLevel, }; -type SharedQueue = { - pending: Update | null, -}; - export type UpdateQueue = { baseState: State, - baseQueue: Update | null, - shared: SharedQueue, - effects: Array> | null, + + firstUpdate: Update | null, + lastUpdate: Update | null, + + firstCapturedUpdate: Update | null, + lastCapturedUpdate: Update | null, + + firstEffect: Update | null, + lastEffect: Update | null, + + firstCapturedEffect: Update | null, + lastCapturedEffect: Update | null, }; export const UpdateState = 0; @@ -155,14 +161,17 @@ if (__DEV__) { }; } -function createUpdateQueue(fiber: Fiber): UpdateQueue { +export function createUpdateQueue(baseState: State): UpdateQueue { const queue: UpdateQueue = { - baseState: fiber.memoizedState, - baseQueue: null, - shared: { - pending: null, - }, - effects: null, + baseState, + firstUpdate: null, + lastUpdate: null, + firstCapturedUpdate: null, + lastCapturedUpdate: null, + firstEffect: null, + lastEffect: null, + firstCapturedEffect: null, + lastCapturedEffect: null, }; return queue; } @@ -172,9 +181,19 @@ function cloneUpdateQueue( ): UpdateQueue { const queue: UpdateQueue = { baseState: currentQueue.baseState, - baseQueue: currentQueue.baseQueue, - shared: currentQueue.shared, - effects: null, + firstUpdate: currentQueue.firstUpdate, + lastUpdate: currentQueue.lastUpdate, + + // TODO: With resuming, if we bail out and resuse the child tree, we should + // keep these effects. + firstCapturedUpdate: null, + lastCapturedUpdate: null, + + firstEffect: null, + lastEffect: null, + + firstCapturedEffect: null, + lastCapturedEffect: null, }; return queue; } @@ -191,46 +210,90 @@ export function createUpdate( payload: null, callback: null, - next: (null: any), + next: null, + nextEffect: null, }; - update.next = update; if (__DEV__) { update.priority = getCurrentPriorityLevel(); } return update; } +function appendUpdateToQueue( + queue: UpdateQueue, + update: Update, +) { + // Append the update to the end of the list. + if (queue.lastUpdate === null) { + // Queue is empty + queue.firstUpdate = queue.lastUpdate = update; + } else { + queue.lastUpdate.next = update; + queue.lastUpdate = update; + } +} + export function enqueueUpdate(fiber: Fiber, update: Update) { - let sharedQueue; - let updateQueue = fiber.updateQueue; - if (updateQueue === null) { - const alternate = fiber.alternate; - if (alternate === null) { - updateQueue = createUpdateQueue(fiber); + // Update queues are created lazily. + const alternate = fiber.alternate; + let queue1; + let queue2; + if (alternate === null) { + // There's only one fiber. + queue1 = fiber.updateQueue; + queue2 = null; + if (queue1 === null) { + queue1 = fiber.updateQueue = createUpdateQueue(fiber.memoizedState); + } + } else { + // There are two owners. + queue1 = fiber.updateQueue; + queue2 = alternate.updateQueue; + if (queue1 === null) { + if (queue2 === null) { + // Neither fiber has an update queue. Create new ones. + queue1 = fiber.updateQueue = createUpdateQueue(fiber.memoizedState); + queue2 = alternate.updateQueue = createUpdateQueue( + alternate.memoizedState, + ); + } else { + // Only one fiber has an update queue. Clone to create a new one. + queue1 = fiber.updateQueue = cloneUpdateQueue(queue2); + } } else { - updateQueue = alternate.updateQueue; - if (updateQueue === null) { - updateQueue = alternate.updateQueue = createUpdateQueue(alternate); + if (queue2 === null) { + // Only one fiber has an update queue. Clone to create a new one. + queue2 = alternate.updateQueue = cloneUpdateQueue(queue1); + } else { + // Both owners have an update queue. } } - fiber.updateQueue = updateQueue; } - sharedQueue = updateQueue.shared; - - const pending = sharedQueue.pending; - if (pending === null) { - // This is the first update. Create a circular list. - update.next = update; + if (queue2 === null || queue1 === queue2) { + // There's only a single queue. + appendUpdateToQueue(queue1, update); } else { - update.next = pending.next; - pending.next = update; + // There are two queues. We need to append the update to both queues, + // while accounting for the persistent structure of the list — we don't + // want the same update to be added multiple times. + if (queue1.lastUpdate === null || queue2.lastUpdate === null) { + // One of the queues is not empty. We must add the update to both queues. + appendUpdateToQueue(queue1, update); + appendUpdateToQueue(queue2, update); + } else { + // Both queues are non-empty. The last update is the same in both lists, + // because of structural sharing. So, only append to one of the lists. + appendUpdateToQueue(queue1, update); + // But we still need to update the `lastUpdate` pointer of queue2. + queue2.lastUpdate = update; + } } - sharedQueue.pending = update; if (__DEV__) { if ( fiber.tag === ClassComponent && - currentlyProcessingQueue === sharedQueue && + (currentlyProcessingQueue === queue1 || + (queue2 !== null && currentlyProcessingQueue === queue2)) && !didWarnUpdateInsideUpdate ) { warningWithoutStack( @@ -249,11 +312,12 @@ export function enqueueCapturedUpdate( workInProgress: Fiber, update: Update, ) { - // Captured updates go only on the work-in-progress queue. + // Captured updates go into a separate list, and only on the work-in- + // progress queue. let workInProgressQueue = workInProgress.updateQueue; if (workInProgressQueue === null) { workInProgressQueue = workInProgress.updateQueue = createUpdateQueue( - workInProgress, + workInProgress.memoizedState, ); } else { // TODO: I put this here rather than createWorkInProgress so that we don't @@ -266,13 +330,12 @@ export function enqueueCapturedUpdate( } // Append the update to the end of the list. - const last = workInProgressQueue.baseQueue; - if (last === null) { - workInProgressQueue.baseQueue = update.next = update; - update.next = update; + if (workInProgressQueue.lastCapturedUpdate === null) { + // This is the first render phase update + workInProgressQueue.firstCapturedUpdate = workInProgressQueue.lastCapturedUpdate = update; } else { - update.next = last.next; - last.next = update; + workInProgressQueue.lastCapturedUpdate.next = update; + workInProgressQueue.lastCapturedUpdate = update; } } @@ -376,163 +439,149 @@ export function processUpdateQueue( queue = ensureWorkInProgressQueueIsAClone(workInProgress, queue); if (__DEV__) { - currentlyProcessingQueue = queue.shared; - } - - // The last rebase update that is NOT part of the base state. - let baseQueue = queue.baseQueue; - - // The last pending update that hasn't been processed yet. - let pendingQueue = queue.shared.pending; - if (pendingQueue !== null) { - // We have new updates that haven't been processed yet. - // We'll add them to the base queue. - if (baseQueue !== null) { - // Merge the pending queue and the base queue. - let baseFirst = baseQueue.next; - let pendingFirst = pendingQueue.next; - baseQueue.next = pendingFirst; - pendingQueue.next = baseFirst; - } - - baseQueue = pendingQueue; - - queue.shared.pending = null; - // TODO: Pass `current` as argument - const current = workInProgress.alternate; - if (current !== null) { - const currentQueue = current.updateQueue; - if (currentQueue !== null) { - currentQueue.baseQueue = pendingQueue; - } - } + currentlyProcessingQueue = queue; } // These values may change as we process the queue. - if (baseQueue !== null) { - let first = baseQueue.next; - // Iterate through the list of updates to compute the result. - let newState = queue.baseState; - let newExpirationTime = NoWork; - - let newBaseState = null; - let newBaseQueueFirst = null; - let newBaseQueueLast = null; - - if (first !== null) { - let update = first; - do { - const updateExpirationTime = update.expirationTime; - if (updateExpirationTime < renderExpirationTime) { - // Priority is insufficient. Skip this update. If this is the first - // skipped update, the previous update/state is the new base - // update/state. - const clone: Update = { - expirationTime: update.expirationTime, - suspenseConfig: update.suspenseConfig, - - tag: update.tag, - payload: update.payload, - callback: update.callback, - - next: (null: any), - }; - if (newBaseQueueLast === null) { - newBaseQueueFirst = newBaseQueueLast = clone; - newBaseState = newState; - } else { - newBaseQueueLast = newBaseQueueLast.next = clone; - } - // Update the remaining priority in the queue. - if (updateExpirationTime > newExpirationTime) { - newExpirationTime = updateExpirationTime; - } + let newBaseState = queue.baseState; + let newFirstUpdate = null; + let newExpirationTime = NoWork; + + // Iterate through the list of updates to compute the result. + let update = queue.firstUpdate; + let resultState = newBaseState; + while (update !== null) { + const updateExpirationTime = update.expirationTime; + if (updateExpirationTime < renderExpirationTime) { + // This update does not have sufficient priority. Skip it. + if (newFirstUpdate === null) { + // This is the first skipped update. It will be the first update in + // the new list. + newFirstUpdate = update; + // Since this is the first update that was skipped, the current result + // is the new base state. + newBaseState = resultState; + } + // Since this update will remain in the list, update the remaining + // expiration time. + if (newExpirationTime < updateExpirationTime) { + newExpirationTime = updateExpirationTime; + } + } else { + // This update does have sufficient priority. + + // Mark the event time of this update as relevant to this render pass. + // TODO: This should ideally use the true event time of this update rather than + // its priority which is a derived and not reverseable value. + // TODO: We should skip this update if it was already committed but currently + // we have no way of detecting the difference between a committed and suspended + // update here. + markRenderEventTimeAndConfig(updateExpirationTime, update.suspenseConfig); + + // Process it and compute a new result. + resultState = getStateFromUpdate( + workInProgress, + queue, + update, + resultState, + props, + instance, + ); + const callback = update.callback; + if (callback !== null) { + workInProgress.effectTag |= Callback; + // Set this to null, in case it was mutated during an aborted render. + update.nextEffect = null; + if (queue.lastEffect === null) { + queue.firstEffect = queue.lastEffect = update; } else { - // This update does have sufficient priority. - - if (newBaseQueueLast !== null) { - const clone: Update = { - expirationTime: Sync, // This update is going to be committed so we never want uncommit it. - suspenseConfig: update.suspenseConfig, - - tag: update.tag, - payload: update.payload, - callback: update.callback, - - next: (null: any), - }; - newBaseQueueLast = newBaseQueueLast.next = clone; - } - - // Mark the event time of this update as relevant to this render pass. - // TODO: This should ideally use the true event time of this update rather than - // its priority which is a derived and not reverseable value. - // TODO: We should skip this update if it was already committed but currently - // we have no way of detecting the difference between a committed and suspended - // update here. - markRenderEventTimeAndConfig( - updateExpirationTime, - update.suspenseConfig, - ); - - // Process this update. - newState = getStateFromUpdate( - workInProgress, - queue, - update, - newState, - props, - instance, - ); - const callback = update.callback; - if (callback !== null) { - workInProgress.effectTag |= Callback; - let effects = queue.effects; - if (effects === null) { - queue.effects = [update]; - } else { - effects.push(update); - } - } + queue.lastEffect.nextEffect = update; + queue.lastEffect = update; } - update = update.next; - if (update === null || update === first) { - pendingQueue = queue.shared.pending; - if (pendingQueue === null) { - break; - } else { - // An update was scheduled from inside a reducer. Add the new - // pending updates to the end of the list and keep processing. - update = baseQueue.next = pendingQueue.next; - pendingQueue.next = first; - queue.baseQueue = baseQueue = pendingQueue; - queue.shared.pending = null; - } - } - } while (true); + } } + // Continue to the next update. + update = update.next; + } - if (newBaseQueueLast === null) { - newBaseState = newState; + // Separately, iterate though the list of captured updates. + let newFirstCapturedUpdate = null; + update = queue.firstCapturedUpdate; + while (update !== null) { + const updateExpirationTime = update.expirationTime; + if (updateExpirationTime < renderExpirationTime) { + // This update does not have sufficient priority. Skip it. + if (newFirstCapturedUpdate === null) { + // This is the first skipped captured update. It will be the first + // update in the new list. + newFirstCapturedUpdate = update; + // If this is the first update that was skipped, the current result is + // the new base state. + if (newFirstUpdate === null) { + newBaseState = resultState; + } + } + // Since this update will remain in the list, update the remaining + // expiration time. + if (newExpirationTime < updateExpirationTime) { + newExpirationTime = updateExpirationTime; + } } else { - newBaseQueueLast.next = (newBaseQueueFirst: any); + // This update does have sufficient priority. Process it and compute + // a new result. + resultState = getStateFromUpdate( + workInProgress, + queue, + update, + resultState, + props, + instance, + ); + const callback = update.callback; + if (callback !== null) { + workInProgress.effectTag |= Callback; + // Set this to null, in case it was mutated during an aborted render. + update.nextEffect = null; + if (queue.lastCapturedEffect === null) { + queue.firstCapturedEffect = queue.lastCapturedEffect = update; + } else { + queue.lastCapturedEffect.nextEffect = update; + queue.lastCapturedEffect = update; + } + } } + update = update.next; + } - queue.baseState = ((newBaseState: any): State); - queue.baseQueue = newBaseQueueLast; - - // Set the remaining expiration time to be whatever is remaining in the queue. - // This should be fine because the only two other things that contribute to - // expiration time are props and context. We're already in the middle of the - // begin phase by the time we start processing the queue, so we've already - // dealt with the props. Context in components that specify - // shouldComponentUpdate is tricky; but we'll have to account for - // that regardless. - markUnprocessedUpdateTime(newExpirationTime); - workInProgress.expirationTime = newExpirationTime; - workInProgress.memoizedState = newState; + if (newFirstUpdate === null) { + queue.lastUpdate = null; + } + if (newFirstCapturedUpdate === null) { + queue.lastCapturedUpdate = null; + } else { + workInProgress.effectTag |= Callback; + } + if (newFirstUpdate === null && newFirstCapturedUpdate === null) { + // We processed every update, without skipping. That means the new base + // state is the same as the result state. + newBaseState = resultState; } + queue.baseState = newBaseState; + queue.firstUpdate = newFirstUpdate; + queue.firstCapturedUpdate = newFirstCapturedUpdate; + + // Set the remaining expiration time to be whatever is remaining in the queue. + // This should be fine because the only two other things that contribute to + // expiration time are props and context. We're already in the middle of the + // begin phase by the time we start processing the queue, so we've already + // dealt with the props. Context in components that specify + // shouldComponentUpdate is tricky; but we'll have to account for + // that regardless. + markUnprocessedUpdateTime(newExpirationTime); + workInProgress.expirationTime = newExpirationTime; + workInProgress.memoizedState = resultState; + if (__DEV__) { currentlyProcessingQueue = null; } @@ -562,17 +611,38 @@ export function commitUpdateQueue( instance: any, renderExpirationTime: ExpirationTime, ): void { + // If the finished render included captured updates, and there are still + // lower priority updates left over, we need to keep the captured updates + // in the queue so that they are rebased and not dropped once we process the + // queue again at the lower priority. + if (finishedQueue.firstCapturedUpdate !== null) { + // Join the captured update list to the end of the normal list. + if (finishedQueue.lastUpdate !== null) { + finishedQueue.lastUpdate.next = finishedQueue.firstCapturedUpdate; + finishedQueue.lastUpdate = finishedQueue.lastCapturedUpdate; + } + // Clear the list of captured updates. + finishedQueue.firstCapturedUpdate = finishedQueue.lastCapturedUpdate = null; + } + // Commit the effects - const effects = finishedQueue.effects; - finishedQueue.effects = null; - if (effects !== null) { - for (let i = 0; i < effects.length; i++) { - const effect = effects[i]; - const callback = effect.callback; - if (callback !== null) { - effect.callback = null; - callCallback(callback, instance); - } + commitUpdateEffects(finishedQueue.firstEffect, instance); + finishedQueue.firstEffect = finishedQueue.lastEffect = null; + + commitUpdateEffects(finishedQueue.firstCapturedEffect, instance); + finishedQueue.firstCapturedEffect = finishedQueue.lastCapturedEffect = null; +} + +function commitUpdateEffects( + effect: Update | null, + instance: any, +): void { + while (effect !== null) { + const callback = effect.callback; + if (callback !== null) { + effect.callback = null; + callCallback(callback, instance); } + effect = effect.nextEffect; } } diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js index 7d199e2522d9..7ae9874105dc 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js @@ -653,176 +653,4 @@ describe('ReactIncrementalUpdates', () => { expect(Scheduler).toFlushAndYield(['Commit: goodbye']); }); }); - - it('when rebasing, does not exclude updates that were already committed, regardless of priority', async () => { - const {useState, useLayoutEffect} = React; - - let pushToLog; - function App() { - const [log, setLog] = useState(''); - pushToLog = msg => { - setLog(prevLog => prevLog + msg); - }; - - useLayoutEffect( - () => { - Scheduler.unstable_yieldValue('Committed: ' + log); - if (log === 'B') { - // Right after B commits, schedule additional updates. - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => { - pushToLog('C'); - }, - ); - setLog(prevLog => prevLog + 'D'); - } - }, - [log], - ); - - return log; - } - - const root = ReactNoop.createRoot(); - await ReactNoop.act(async () => { - root.render(); - }); - expect(Scheduler).toHaveYielded(['Committed: ']); - expect(root).toMatchRenderedOutput(''); - - await ReactNoop.act(async () => { - pushToLog('A'); - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => { - pushToLog('B'); - }, - ); - }); - expect(Scheduler).toHaveYielded([ - // A and B are pending. B is higher priority, so we'll render that first. - 'Committed: B', - // Because A comes first in the queue, we're now in rebase mode. B must - // be rebased on top of A. Also, in a layout effect, we received two new - // updates: C and D. C is user-blocking and D is synchronous. - // - // First render the synchronous update. What we're testing here is that - // B *is not dropped* even though it has lower than sync priority. That's - // because we already committed it. However, this render should not - // include C, because that update wasn't already committed. - 'Committed: BD', - 'Committed: BCD', - 'Committed: ABCD', - ]); - expect(root).toMatchRenderedOutput('ABCD'); - }); - - it('when rebasing, does not exclude updates that were already committed, regardless of priority (classes)', async () => { - let pushToLog; - class App extends React.Component { - state = {log: ''}; - pushToLog = msg => { - this.setState(prevState => ({log: prevState.log + msg})); - }; - componentDidUpdate() { - Scheduler.unstable_yieldValue('Committed: ' + this.state.log); - if (this.state.log === 'B') { - // Right after B commits, schedule additional updates. - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => { - this.pushToLog('C'); - }, - ); - this.pushToLog('D'); - } - } - render() { - pushToLog = this.pushToLog; - return this.state.log; - } - } - - const root = ReactNoop.createRoot(); - await ReactNoop.act(async () => { - root.render(); - }); - expect(Scheduler).toHaveYielded([]); - expect(root).toMatchRenderedOutput(''); - - await ReactNoop.act(async () => { - pushToLog('A'); - Scheduler.unstable_runWithPriority( - Scheduler.unstable_UserBlockingPriority, - () => { - pushToLog('B'); - }, - ); - }); - expect(Scheduler).toHaveYielded([ - // A and B are pending. B is higher priority, so we'll render that first. - 'Committed: B', - // Because A comes first in the queue, we're now in rebase mode. B must - // be rebased on top of A. Also, in a layout effect, we received two new - // updates: C and D. C is user-blocking and D is synchronous. - // - // First render the synchronous update. What we're testing here is that - // B *is not dropped* even though it has lower than sync priority. That's - // because we already committed it. However, this render should not - // include C, because that update wasn't already committed. - 'Committed: BD', - 'Committed: BCD', - 'Committed: ABCD', - ]); - expect(root).toMatchRenderedOutput('ABCD'); - }); - - it("base state of update queue is initialized to its fiber's memoized state", async () => { - // This test is very weird because it tests an implementation detail but - // is tested in terms of public APIs. When it was originally written, the - // test failed because the update queue was initialized to the state of - // the alternate fiber. - let app; - class App extends React.Component { - state = {prevProp: 'A', count: 0}; - static getDerivedStateFromProps(props, state) { - // Add 100 whenever the label prop changes. The prev label is stored - // in state. If the state is dropped incorrectly, we'll fail to detect - // prop changes. - if (props.prop !== state.prevProp) { - return { - prevProp: props.prop, - count: state.count + 100, - }; - } - return null; - } - render() { - app = this; - return this.state.count; - } - } - - const root = ReactNoop.createRoot(); - await ReactNoop.act(async () => { - root.render(); - }); - expect(root).toMatchRenderedOutput('0'); - - // Changing the prop causes the count to increase by 100 - await ReactNoop.act(async () => { - root.render(); - }); - expect(root).toMatchRenderedOutput('100'); - - // Now increment the count by 1 with a state update. And, in the same - // batch, change the prop back to its original value. - await ReactNoop.act(async () => { - root.render(); - app.setState(state => ({count: state.count + 1})); - }); - // There were two total prop changes, plus an increment. - expect(root).toMatchRenderedOutput('201'); - }); });