diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 24a368056c0a..61a14a94af4d 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -1142,20 +1142,33 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { function logUpdateQueue(updateQueue: UpdateQueue, depth) { log(' '.repeat(depth + 1) + 'QUEUED UPDATES'); - const firstUpdate = updateQueue.firstUpdate; - if (!firstUpdate) { + const last = updateQueue.baseQueue; + if (last === null) { return; } + const first = last.next; + let update = first; + if (update !== null) { + do { + log( + ' '.repeat(depth + 1) + '~', + '[' + update.expirationTime + ']', + ); + } while (update !== null && update !== first); + } - log( - ' '.repeat(depth + 1) + '~', - '[' + firstUpdate.expirationTime + ']', - ); - while (firstUpdate.next) { - log( - ' '.repeat(depth + 1) + '~', - '[' + firstUpdate.expirationTime + ']', - ); + 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); + } } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 678e1398d39c..3d3342e9ae49 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -90,7 +90,11 @@ import { reconcileChildFibers, cloneChildFibers, } from './ReactChildFiber'; -import {processUpdateQueue} from './ReactUpdateQueue'; +import { + processUpdateQueue, + cloneUpdateQueue, + initializeUpdateQueue, +} from './ReactUpdateQueue'; import { NoWork, Never, @@ -904,7 +908,7 @@ function updateHostRoot(current, workInProgress, renderExpirationTime) { pushHostRootContext(workInProgress); const updateQueue = workInProgress.updateQueue; invariant( - updateQueue !== null, + current !== null && updateQueue !== null, 'If the root does not have an updateQueue, we should have already ' + 'bailed out. This error is likely caused by a bug in React. Please ' + 'file an issue.', @@ -912,13 +916,8 @@ function updateHostRoot(current, workInProgress, renderExpirationTime) { const nextProps = workInProgress.pendingProps; const prevState = workInProgress.memoizedState; const prevChildren = prevState !== null ? prevState.element : null; - processUpdateQueue( - workInProgress, - updateQueue, - nextProps, - null, - renderExpirationTime, - ); + cloneUpdateQueue(current, workInProgress); + processUpdateQueue(workInProgress, nextProps, null, renderExpirationTime); const nextState = workInProgress.memoizedState; // Caution: React DevTools currently depends on this property // being called "element". @@ -1338,6 +1337,8 @@ function mountIndeterminateComponent( workInProgress.memoizedState = value.state !== null && value.state !== undefined ? value.state : null; + initializeUpdateQueue(workInProgress); + const getDerivedStateFromProps = Component.getDerivedStateFromProps; if (typeof getDerivedStateFromProps === 'function') { applyDerivedStateFromProps( diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 8dc7e45835ea..5d0d05bf9e91 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -9,6 +9,7 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; +import type {UpdateQueue} from './ReactUpdateQueue'; import React from 'react'; import {Update, Snapshot} from 'shared/ReactSideEffectTags'; @@ -38,6 +39,8 @@ import { createUpdate, ReplaceState, ForceUpdate, + initializeUpdateQueue, + cloneUpdateQueue, } from './ReactUpdateQueue'; import {NoWork} from './ReactFiberExpirationTime'; import { @@ -171,8 +174,9 @@ export function applyDerivedStateFromProps( // Once the update queue is empty, persist the derived state onto the // base state. - const updateQueue = workInProgress.updateQueue; - if (updateQueue !== null && workInProgress.expirationTime === NoWork) { + if (workInProgress.expirationTime === NoWork) { + // Queue is always non-null for classes + const updateQueue: UpdateQueue = (workInProgress.updateQueue: any); updateQueue.baseState = memoizedState; } } @@ -789,6 +793,8 @@ function mountClassInstance( instance.state = workInProgress.memoizedState; instance.refs = emptyRefsObject; + initializeUpdateQueue(workInProgress); + const contextType = ctor.contextType; if (typeof contextType === 'object' && contextType !== null) { instance.context = readContext(contextType); @@ -829,17 +835,8 @@ function mountClassInstance( } } - let updateQueue = workInProgress.updateQueue; - if (updateQueue !== null) { - processUpdateQueue( - workInProgress, - updateQueue, - newProps, - instance, - renderExpirationTime, - ); - instance.state = workInProgress.memoizedState; - } + processUpdateQueue(workInProgress, newProps, instance, renderExpirationTime); + instance.state = workInProgress.memoizedState; const getDerivedStateFromProps = ctor.getDerivedStateFromProps; if (typeof getDerivedStateFromProps === 'function') { @@ -863,17 +860,13 @@ function mountClassInstance( callComponentWillMount(workInProgress, instance); // If we had additional state updates during this life-cycle, let's // process them now. - updateQueue = workInProgress.updateQueue; - if (updateQueue !== null) { - processUpdateQueue( - workInProgress, - updateQueue, - newProps, - instance, - renderExpirationTime, - ); - instance.state = workInProgress.memoizedState; - } + processUpdateQueue( + workInProgress, + newProps, + instance, + renderExpirationTime, + ); + instance.state = workInProgress.memoizedState; } if (typeof instance.componentDidMount === 'function') { @@ -936,17 +929,8 @@ function resumeMountClassInstance( const oldState = workInProgress.memoizedState; let newState = (instance.state = oldState); - let updateQueue = workInProgress.updateQueue; - if (updateQueue !== null) { - processUpdateQueue( - workInProgress, - updateQueue, - newProps, - instance, - renderExpirationTime, - ); - newState = workInProgress.memoizedState; - } + processUpdateQueue(workInProgress, newProps, instance, renderExpirationTime); + newState = workInProgress.memoizedState; if ( oldProps === newProps && oldState === newState && @@ -1035,6 +1019,8 @@ function updateClassInstance( ): boolean { const instance = workInProgress.stateNode; + cloneUpdateQueue(current, workInProgress); + const oldProps = workInProgress.memoizedProps; instance.props = workInProgress.type === workInProgress.elementType @@ -1081,17 +1067,8 @@ function updateClassInstance( const oldState = workInProgress.memoizedState; let newState = (instance.state = oldState); - let updateQueue = workInProgress.updateQueue; - if (updateQueue !== null) { - processUpdateQueue( - workInProgress, - updateQueue, - newProps, - instance, - renderExpirationTime, - ); - newState = workInProgress.memoizedState; - } + processUpdateQueue(workInProgress, newProps, instance, renderExpirationTime); + newState = workInProgress.memoizedState; if ( oldProps === newProps && diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 7a4c7dfae82b..0a6c3a132cc2 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} from './ReactFiberExpirationTime'; +import {NoWork, Sync} 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 | null, + next: Update, priority?: ReactPriorityLevel, }; type UpdateQueue = { - last: Update | null, + pending: 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, - baseUpdate: Update | null, + baseQueue: 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 = { - last: null, + pending: 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.baseUpdate === queue.last) { + if (hook.baseQueue === null) { hook.baseState = newState; } @@ -715,42 +715,55 @@ function updateReducer( return [hook.memoizedState, dispatch]; } - // 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; + 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; } - first = baseUpdate.next; - } else { - first = last !== null ? last.next : null; + current.baseQueue = baseQueue = pendingQueue; + queue.pending = null; } - if (first !== null) { - let newState = baseState; + + if (baseQueue !== null) { + // We have a queue to process. + let first = baseQueue.next; + let newState = current.baseState; + let newBaseState = null; - let newBaseUpdate = null; - let prevUpdate = baseUpdate; + let newBaseQueueFirst = null; + let newBaseQueueLast = null; 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. - if (!didSkip) { - didSkip = true; - newBaseUpdate = prevUpdate; + 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; newBaseState = newState; + } else { + newBaseQueueLast = newBaseQueueLast.next = clone; } // Update the remaining priority in the queue. if (updateExpirationTime > currentlyRenderingFiber.expirationTime) { @@ -760,6 +773,18 @@ 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. @@ -781,13 +806,13 @@ function updateReducer( newState = reducer(newState, action); } } - prevUpdate = update; update = update.next; } while (update !== null && update !== first); - if (!didSkip) { - newBaseUpdate = prevUpdate; + if (newBaseQueueLast === null) { newBaseState = newState; + } else { + newBaseQueueLast.next = (newBaseQueueFirst: any); } // Mark that the fiber performed work, but only if the new state is @@ -797,8 +822,8 @@ function updateReducer( } hook.memoizedState = newState; - hook.baseUpdate = newBaseUpdate; hook.baseState = newBaseState; + hook.baseQueue = newBaseQueueLast; queue.lastRenderedState = newState; } @@ -816,7 +841,7 @@ function mountState( } hook.memoizedState = hook.baseState = initialState; const queue = (hook.queue = { - last: null, + pending: null, dispatch: null, lastRenderedReducer: basicStateReducer, lastRenderedState: (initialState: any), @@ -1233,7 +1258,7 @@ function dispatchAction( action, eagerReducer: null, eagerState: null, - next: null, + next: (null: any), }; if (__DEV__) { update.priority = getCurrentPriorityLevel(); @@ -1267,7 +1292,7 @@ function dispatchAction( action, eagerReducer: null, eagerState: null, - next: null, + next: (null: any), }; if (__DEV__) { @@ -1275,19 +1300,15 @@ function dispatchAction( } // Append the update to the end of the list. - const last = queue.last; - if (last === null) { + const pending = queue.pending; + if (pending === null) { // This is the first update. Create a circular list. update.next = update; } else { - const first = last.next; - if (first !== null) { - // Still circular. - update.next = first; - } - last.next = update; + update.next = pending.next; + pending.next = update; } - queue.last = update; + queue.pending = update; if ( fiber.expirationTime === NoWork && diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 3f82c22f8787..f1314e310801 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -25,6 +25,7 @@ import { } from 'shared/ReactFeatureFlags'; import {unstable_getThreadID} from 'scheduler/tracing'; import {NoPriority} from './SchedulerWithReactIntegration'; +import {initializeUpdateQueue} from './ReactUpdateQueue'; export type PendingInteractionMap = Map>; @@ -149,6 +150,8 @@ export function createFiberRoot( root.current = uninitializedFiber; uninitializedFiber.stateNode = root; + initializeUpdateQueue(uninitializedFiber); + return root; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 8273d92b3c7a..649f37a410ab 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -14,6 +14,7 @@ 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, @@ -2859,7 +2860,7 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) { // has triggered any high priority updates const updateQueue = current.updateQueue; if (updateQueue !== null) { - let update = updateQueue.firstUpdate; + let update = updateQueue.baseQueue; while (update !== null) { const priorityLevel = update.priority; if ( @@ -2883,12 +2884,11 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) { break; case FunctionComponent: case ForwardRef: - case SimpleMemoComponent: - if ( - workInProgressNode.memoizedState !== null && - workInProgressNode.memoizedState.baseUpdate !== null - ) { - let update = workInProgressNode.memoizedState.baseUpdate; + 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; // Loop through the functional component's memoized state to see whether // the component has triggered any high pri updates while (update !== null) { @@ -2908,15 +2908,14 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) { } break; } - if ( - update.next === workInProgressNode.memoizedState.baseUpdate - ) { + if (update.next === firstHook.baseQueue) { break; } update = update.next; } } break; + } default: break; } diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index 0d2a8f68230b..3cdf29b74efa 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -89,13 +89,12 @@ import type {ExpirationTime} from './ReactFiberExpirationTime'; import type {SuspenseConfig} from './ReactFiberSuspenseConfig'; import type {ReactPriorityLevel} from './SchedulerWithReactIntegration'; -import {NoWork} from './ReactFiberExpirationTime'; +import {NoWork, Sync} from './ReactFiberExpirationTime'; import { enterDisallowedContextReadInDEV, exitDisallowedContextReadInDEV, } from './ReactFiberNewContext'; import {Callback, ShouldCapture, DidCapture} from 'shared/ReactSideEffectTags'; -import {ClassComponent} from 'shared/ReactWorkTags'; import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags'; @@ -117,27 +116,21 @@ export type Update = { payload: any, callback: (() => mixed) | null, - next: Update | null, - nextEffect: Update | null, + next: Update, //DEV only priority?: ReactPriorityLevel, }; +type SharedQueue = { + pending: Update | null, +}; + export type UpdateQueue = { baseState: State, - - firstUpdate: Update | null, - lastUpdate: Update | null, - - firstCapturedUpdate: Update | null, - lastCapturedUpdate: Update | null, - - firstEffect: Update | null, - lastEffect: Update | null, - - firstCapturedEffect: Update | null, - lastCapturedEffect: Update | null, + baseQueue: Update | null, + shared: SharedQueue, + effects: Array> | null, }; export const UpdateState = 0; @@ -161,41 +154,34 @@ if (__DEV__) { }; } -export function createUpdateQueue(baseState: State): UpdateQueue { +export function initializeUpdateQueue(fiber: Fiber): void { const queue: UpdateQueue = { - baseState, - firstUpdate: null, - lastUpdate: null, - firstCapturedUpdate: null, - lastCapturedUpdate: null, - firstEffect: null, - lastEffect: null, - firstCapturedEffect: null, - lastCapturedEffect: null, + baseState: fiber.memoizedState, + baseQueue: null, + shared: { + pending: null, + }, + effects: null, }; - return queue; + fiber.updateQueue = queue; } -function cloneUpdateQueue( - currentQueue: UpdateQueue, -): UpdateQueue { - const queue: UpdateQueue = { - baseState: currentQueue.baseState, - 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; +export function cloneUpdateQueue( + current: Fiber, + workInProgress: Fiber, +): void { + // Clone the update queue from current. Unless it's already a clone. + const queue: UpdateQueue = (workInProgress.updateQueue: any); + const currentQueue: UpdateQueue = (current.updateQueue: any); + if (queue === currentQueue) { + const clone: UpdateQueue = { + baseState: currentQueue.baseState, + baseQueue: currentQueue.baseQueue, + shared: currentQueue.shared, + effects: currentQueue.effects, + }; + workInProgress.updateQueue = clone; + } } export function createUpdate( @@ -210,90 +196,36 @@ export function createUpdate( payload: null, callback: null, - next: null, - nextEffect: null, + next: (null: any), }; + 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) { - // 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 { - 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. - } - } + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { + // Only occurs if the fiber has been unmounted. + return; } - if (queue2 === null || queue1 === queue2) { - // There's only a single queue. - appendUpdateToQueue(queue1, update); + + const sharedQueue = updateQueue.shared; + const pending = sharedQueue.pending; + if (pending === null) { + // This is the first update. Create a circular list. + update.next = update; } else { - // 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; - } + update.next = pending.next; + pending.next = update; } + sharedQueue.pending = update; if (__DEV__) { if ( - fiber.tag === ClassComponent && - (currentlyProcessingQueue === queue1 || - (queue2 !== null && currentlyProcessingQueue === queue2)) && + currentlyProcessingQueue === sharedQueue && !didWarnUpdateInsideUpdate ) { warningWithoutStack( @@ -312,48 +244,25 @@ export function enqueueCapturedUpdate( workInProgress: Fiber, update: Update, ) { - // 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.memoizedState, - ); - } else { - // TODO: I put this here rather than createWorkInProgress so that we don't - // clone the queue unnecessarily. There's probably a better way to - // structure this. - workInProgressQueue = ensureWorkInProgressQueueIsAClone( - workInProgress, - workInProgressQueue, - ); + const current = workInProgress.alternate; + if (current !== null) { + // Ensure the work-in-progress queue is a clone + cloneUpdateQueue(current, workInProgress); } + // Captured updates go only on the work-in-progress queue. + const queue: UpdateQueue = (workInProgress.updateQueue: any); // Append the update to the end of the list. - if (workInProgressQueue.lastCapturedUpdate === null) { - // This is the first render phase update - workInProgressQueue.firstCapturedUpdate = workInProgressQueue.lastCapturedUpdate = update; + const last = queue.baseQueue; + if (last === null) { + queue.baseQueue = update.next = update; + update.next = update; } else { - workInProgressQueue.lastCapturedUpdate.next = update; - workInProgressQueue.lastCapturedUpdate = update; + update.next = last.next; + last.next = update; } } -function ensureWorkInProgressQueueIsAClone( - workInProgress: Fiber, - queue: UpdateQueue, -): UpdateQueue { - const current = workInProgress.alternate; - if (current !== null) { - // If the work-in-progress queue is equal to the current queue, - // we need to clone it first. - if (queue === current.updateQueue) { - queue = workInProgress.updateQueue = cloneUpdateQueue(queue); - } - } - return queue; -} - function getStateFromUpdate( workInProgress: Fiber, queue: UpdateQueue, @@ -429,158 +338,172 @@ function getStateFromUpdate( export function processUpdateQueue( workInProgress: Fiber, - queue: UpdateQueue, props: any, instance: any, renderExpirationTime: ExpirationTime, ): void { - hasForceUpdate = false; + // This is always non-null on a ClassComponent or HostRoot + const queue: UpdateQueue = (workInProgress.updateQueue: any); - queue = ensureWorkInProgressQueueIsAClone(workInProgress, queue); + hasForceUpdate = false; if (__DEV__) { - currentlyProcessingQueue = queue; + currentlyProcessingQueue = queue.shared; } - // These values may change as we process the queue. - 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 { - queue.lastEffect.nextEffect = update; - queue.lastEffect = update; - } + // 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; } } - // Continue to the next update. - update = update.next; } - // 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 { - // 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; + // 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; + } } else { - queue.lastCapturedEffect.nextEffect = update; - queue.lastCapturedEffect = update; + // 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); + } + } } - } + 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); } - update = update.next; - } - 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; - } + if (newBaseQueueLast === null) { + newBaseState = newState; + } else { + newBaseQueueLast.next = (newBaseQueueFirst: any); + } - 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; + 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 (__DEV__) { currentlyProcessingQueue = null; @@ -611,38 +534,17 @@ 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 - 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); + 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); + } } - 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 7ae9874105dc..7d199e2522d9 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js @@ -653,4 +653,176 @@ 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'); + }); });