From e6eb32385ee6b5267e8634b3dae17479bbc3e6c9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 9 Dec 2019 15:09:48 -0800 Subject: [PATCH] Initialize update queue object on mount Instead of lazily initializing update queue objects on the first update, class and host root queues are created on mount. This simplifies the logic for appending new updates and matches what we do for hooks. --- .../src/ReactFiberBeginWork.js | 19 ++-- .../src/ReactFiberClassComponent.js | 69 +++++--------- .../react-reconciler/src/ReactFiberRoot.js | 3 + .../react-reconciler/src/ReactUpdateQueue.js | 93 +++++++------------ 4 files changed, 69 insertions(+), 115 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 678e1398d39c1..3d3342e9ae49b 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 8dc7e45835ea5..5d0d05bf9e91f 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/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 3f82c22f87871..f1314e3108013 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/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index 10c7e55274e8c..d58d245fe620b 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -155,7 +155,7 @@ if (__DEV__) { }; } -function createUpdateQueue(fiber: Fiber): UpdateQueue { +export function initializeUpdateQueue(fiber: Fiber): void { const queue: UpdateQueue = { baseState: fiber.memoizedState, baseQueue: null, @@ -164,19 +164,25 @@ function createUpdateQueue(fiber: Fiber): UpdateQueue { }, effects: null, }; - return queue; + fiber.updateQueue = queue; } -function cloneUpdateQueue( - currentQueue: UpdateQueue, -): UpdateQueue { - const queue: UpdateQueue = { - baseState: currentQueue.baseState, - baseQueue: currentQueue.baseQueue, - shared: currentQueue.shared, - effects: 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( @@ -201,22 +207,13 @@ export function createUpdate( } export function enqueueUpdate(fiber: Fiber, update: Update) { - let sharedQueue; - let updateQueue = fiber.updateQueue; + const updateQueue = fiber.updateQueue; if (updateQueue === null) { - const alternate = fiber.alternate; - if (alternate === null) { - updateQueue = createUpdateQueue(fiber); - } else { - updateQueue = alternate.updateQueue; - if (updateQueue === null) { - updateQueue = alternate.updateQueue = createUpdateQueue(alternate); - } - } - fiber.updateQueue = updateQueue; + // Only occurs if the fiber has been unmounted. + return; } - sharedQueue = updateQueue.shared; + const sharedQueue = updateQueue.shared; const pending = sharedQueue.pending; if (pending === null) { // This is the first update. Create a circular list. @@ -229,7 +226,6 @@ export function enqueueUpdate(fiber: Fiber, update: Update) { if (__DEV__) { if ( - fiber.tag === ClassComponent && currentlyProcessingQueue === sharedQueue && !didWarnUpdateInsideUpdate ) { @@ -249,26 +245,18 @@ export function enqueueCapturedUpdate( workInProgress: Fiber, update: Update, ) { - // Captured updates go only on the work-in-progress queue. - let workInProgressQueue = workInProgress.updateQueue; - if (workInProgressQueue === null) { - workInProgressQueue = workInProgress.updateQueue = createUpdateQueue( - workInProgress, - ); - } 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. - const last = workInProgressQueue.baseQueue; + const last = queue.baseQueue; if (last === null) { - workInProgressQueue.baseQueue = update.next = update; + queue.baseQueue = update.next = update; update.next = update; } else { update.next = last.next; @@ -276,21 +264,6 @@ export function enqueueCapturedUpdate( } } -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, @@ -366,14 +339,14 @@ 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.shared;