From 89ca3cf3019abee3d320bd15b5db6c893229ec94 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 27 Nov 2019 22:06:47 -0800 Subject: [PATCH] Refactor Hooks update queue --- .../react-reconciler/src/ReactFiberHooks.js | 119 ++++++++++-------- .../src/ReactFiberWorkLoop.js | 17 ++- 2 files changed, 78 insertions(+), 58 deletions(-) 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/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 8273d92b3c7a..8d0ced43911b 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, @@ -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; }