From cbba5ef9cc46564cd8afb981a98b33fe5b62e2e6 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 9 Dec 2016 00:32:03 -0800 Subject: [PATCH 01/16] Don't drop updates until they are committed Restructures the update queue to maintain a pointer to the first pending update, which solves a few problems: - Updates that occur during the begin phase (e.g. in cWRP of a child) aren't dropped, like they are currently. This isn't working yet because the work priority is reset during completion. The following item will fix it. - Sets us up to be able to add separate priorities to each update in the queue. I'll add this in a subsequent commit. --- src/renderers/noop/ReactNoop.js | 14 +- src/renderers/shared/fiber/ReactFiber.js | 11 +- .../shared/fiber/ReactFiberBeginWork.js | 67 +++- .../shared/fiber/ReactFiberClassComponent.js | 49 ++- .../shared/fiber/ReactFiberCommitWork.js | 24 +- .../shared/fiber/ReactFiberCompleteWork.js | 28 +- .../shared/fiber/ReactFiberReconciler.js | 55 ++-- src/renderers/shared/fiber/ReactFiberRoot.js | 3 - .../shared/fiber/ReactFiberScheduler.js | 1 - .../shared/fiber/ReactFiberUpdateQueue.js | 306 +++++++++++++----- 10 files changed, 363 insertions(+), 195 deletions(-) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index d16b6e7bf2..c3ca7bc633 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -284,17 +284,19 @@ var ReactNoop = { function logUpdateQueue(updateQueue : UpdateQueue, depth) { log( - ' '.repeat(depth + 1) + 'QUEUED UPDATES', - updateQueue.isReplace ? 'is replace' : '', - updateQueue.isForced ? 'is forced' : '' + ' '.repeat(depth + 1) + 'QUEUED UPDATES' ); + const firstPendingUpdate = updateQueue.firstPendingUpdate; + if (!firstPendingUpdate) { + return; + } log( ' '.repeat(depth + 1) + '~', - updateQueue.partialState, - updateQueue.callback ? 'with callback' : '' + firstPendingUpdate && firstPendingUpdate.partialState, + firstPendingUpdate.callback ? 'with callback' : '' ); var next; - while (next = updateQueue.next) { + while (next = firstPendingUpdate.next) { log( ' '.repeat(depth + 1) + '~', next.partialState, diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 8873e2b46c..5145e1a995 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -100,12 +100,11 @@ export type Fiber = { pendingProps: any, // This type will be more specific once we overload the tag. // TODO: I think that there is a way to merge pendingProps and memoizedProps. memoizedProps: any, // The props used to create the output. - // A queue of local state updates. - updateQueue: ?UpdateQueue, - // The state used to create the output. This is a full state object. + + // A queue of state updates and callbacks. + updateQueue: UpdateQueue | null, + // The state used to create the output memoizedState: any, - // Linked list of callbacks to call after updates are committed. - callbackList: ?UpdateQueue, // Effect effectTag: TypeOfSideEffect, @@ -195,7 +194,6 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { memoizedProps: null, updateQueue: null, memoizedState: null, - callbackList: null, effectTag: NoEffect, nextEffect: null, @@ -271,7 +269,6 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; alt.updateQueue = fiber.updateQueue; - alt.callbackList = fiber.callbackList; alt.pendingWorkPriority = priorityLevel; alt.memoizedProps = fiber.memoizedProps; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index bc98949e6f..fcc89e0532 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -25,7 +25,10 @@ var { reconcileChildFibersInPlace, cloneChildFibers, } = require('ReactChildFiber'); - +var { + hasPendingUpdate, + mergeQueue, +} = require('ReactFiberUpdateQueue'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { getMaskedContext, @@ -53,6 +56,8 @@ var { OffscreenPriority, } = require('ReactPriorityLevel'); var { + Update, + Callback, Placement, ContentReset, Err, @@ -210,9 +215,29 @@ module.exports = function( } else { shouldUpdate = updateClassInstance(current, workInProgress); } - if (!shouldUpdate) { + + // Schedule side-effects + const updateQueue = workInProgress.updateQueue; + if (updateQueue && updateQueue.hasCallback) { + // The update queue has a callback. Schedule a callback effect. + // Callbacks are scheduled regardless of whether we bail out below. + workInProgress.effectTag |= Callback; + } + if (shouldUpdate) { + workInProgress.effectTag |= Update; + } else { + // If an update was already in progress, we should schedule an Update + // effect even though we're bailing out, so that cWU/cDU are called. + if (current) { + const instance = current.stateNode; + if (instance.props !== current.memoizedProps || + instance.state !== current.memoizedState) { + workInProgress.effectTag |= Update; + } + } return bailoutOnAlreadyFinishedWork(current, workInProgress); } + // Rerender const instance = workInProgress.stateNode; ReactCurrentOwner.current = workInProgress; @@ -471,13 +496,24 @@ module.exports = function( workInProgress.child = workInProgress.progressedChild; } - if ((workInProgress.pendingProps === null || ( - workInProgress.memoizedProps !== null && - workInProgress.pendingProps === workInProgress.memoizedProps - )) && - workInProgress.updateQueue === null && - !hasContextChanged()) { - return bailoutOnAlreadyFinishedWork(current, workInProgress); + const pendingProps = workInProgress.pendingProps; + const memoizedProps = workInProgress.memoizedProps; + const updateQueue = workInProgress.updateQueue; + + + + // This is kept as a single expression to take advantage of short-circuiting. + const hasNewProps = ( + pendingProps !== null && ( // hasPendingProps && ( + memoizedProps === null || // hasNoMemoizedProps || + pendingProps !== memoizedProps // memoizedPropsDontMatch + ) // ) + ); + if (!hasNewProps) { + const hasUpdate = updateQueue && hasPendingUpdate(updateQueue); + if (!hasUpdate && !hasContextChanged()) { + return bailoutOnAlreadyFinishedWork(current, workInProgress); + } } switch (workInProgress.tag) { @@ -497,8 +533,19 @@ module.exports = function( } else { pushTopLevelContextObject(root.context, false); } + + if (updateQueue) { + // The last three arguments are unimportant because there should be + // no update functions in a HostRoot's queue. + mergeQueue(updateQueue, null, null, null); + if (updateQueue.hasCallback) { + workInProgress.effectTag |= Callback; + } + } + pushHostContainer(workInProgress.stateNode.containerInfo); - reconcileChildren(current, workInProgress, workInProgress.pendingProps); + reconcileChildren(current, workInProgress, pendingProps); + // A yield component is just a placeholder, we can just run through the // next one immediately. return workInProgress.child; diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 79595dd152..b72c6bb777 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -20,9 +20,11 @@ var { } = require('ReactFiberContext'); var { createUpdateQueue, - addToQueue, - addCallbackToQueue, - mergeUpdateQueue, + addUpdate, + addReplaceUpdate, + addForceUpdate, + addCallback, + mergeQueue, } = require('ReactFiberUpdateQueue'); var { getComponentName, isMounted } = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); @@ -49,36 +51,33 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { isMounted, enqueueSetState(instance, partialState) { const fiber = ReactInstanceMap.get(instance); - const updateQueue = fiber.updateQueue ? - addToQueue(fiber.updateQueue, partialState) : - createUpdateQueue(partialState); - scheduleUpdateQueue(fiber, updateQueue); + const queue = fiber.updateQueue || createUpdateQueue(); + addUpdate(queue, partialState); + scheduleUpdateQueue(fiber, queue); }, enqueueReplaceState(instance, state) { const fiber = ReactInstanceMap.get(instance); - const updateQueue = createUpdateQueue(state); - updateQueue.isReplace = true; - scheduleUpdateQueue(fiber, updateQueue); + const queue = fiber.updateQueue || createUpdateQueue(); + addReplaceUpdate(queue, state); + scheduleUpdateQueue(fiber, queue); }, enqueueForceUpdate(instance) { const fiber = ReactInstanceMap.get(instance); - const updateQueue = fiber.updateQueue || createUpdateQueue(null); - updateQueue.isForced = true; - scheduleUpdateQueue(fiber, updateQueue); + const queue = fiber.updateQueue || createUpdateQueue(); + addForceUpdate(queue); + scheduleUpdateQueue(fiber, queue); }, enqueueCallback(instance, callback) { const fiber = ReactInstanceMap.get(instance); - let updateQueue = fiber.updateQueue ? - fiber.updateQueue : - createUpdateQueue(null); - addCallbackToQueue(updateQueue, callback); - scheduleUpdateQueue(fiber, updateQueue); + const queue = fiber.updateQueue || createUpdateQueue(); + addCallback(queue, callback); + scheduleUpdateQueue(fiber, queue); }, }; function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState, newContext) { const updateQueue = workInProgress.updateQueue; - if (oldProps === null || (updateQueue && updateQueue.isForced)) { + if (oldProps === null || (updateQueue && updateQueue.hasForceUpdate)) { return true; } @@ -245,7 +244,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // process them now. const updateQueue = workInProgress.updateQueue; if (updateQueue) { - instance.state = mergeUpdateQueue(updateQueue, instance, state, props); + instance.state = mergeQueue(updateQueue, instance, state, props); } } } @@ -294,7 +293,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // during initial mounting. const newUpdateQueue = workInProgress.updateQueue; if (newUpdateQueue) { - newInstance.state = mergeUpdateQueue(newUpdateQueue, newInstance, newState, newProps); + newInstance.state = mergeQueue(newUpdateQueue, newInstance, newState, newProps); } return true; } @@ -332,11 +331,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // TODO: Previous state can be null. let newState; if (updateQueue) { - if (!updateQueue.hasUpdate) { - newState = oldState; - } else { - newState = mergeUpdateQueue(updateQueue, instance, oldState, newProps); - } + newState = mergeQueue(updateQueue, instance, oldState, newProps); } else { newState = oldState; } @@ -344,7 +339,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { if (oldProps === newProps && oldState === newState && oldContext === newContext && - updateQueue && !updateQueue.isForced) { + updateQueue && !updateQueue.hasForceUpdate) { return false; } diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 5ff31e2d51..9c1bf2a9f1 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -25,12 +25,11 @@ var { HostPortal, CoroutineComponent, } = ReactTypeOfWork; -var { callCallbacks } = require('ReactFiberUpdateQueue'); +var { commitUpdateQueue } = require('ReactFiberUpdateQueue'); var { Placement, Update, - Callback, ContentReset, } = require('ReactTypeOfSideEffect'); @@ -418,25 +417,16 @@ module.exports = function( } attachRef(current, finishedWork, instance); } - // Clear updates from current fiber. - if (finishedWork.alternate) { - finishedWork.alternate.updateQueue = null; - } - if (finishedWork.effectTag & Callback) { - if (finishedWork.callbackList) { - const callbackList = finishedWork.callbackList; - finishedWork.callbackList = null; - callCallbacks(callbackList, instance); - } + if (finishedWork.updateQueue) { + commitUpdateQueue(finishedWork, finishedWork.updateQueue, instance); } return; } case HostRoot: { - const rootFiber = finishedWork.stateNode; - if (rootFiber.callbackList) { - const callbackList = rootFiber.callbackList; - rootFiber.callbackList = null; - callCallbacks(callbackList, rootFiber.current.child.stateNode); + const updateQueue = finishedWork.updateQueue; + if (updateQueue) { + const instance = finishedWork.child && finishedWork.child.stateNode; + commitUpdateQueue(finishedWork, updateQueue, instance); } return; } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index d0a6ab217d..93e61ce0c1 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -41,7 +41,6 @@ var { } = ReactTypeOfWork; var { Update, - Callback, } = ReactTypeOfSideEffect; if (__DEV__) { @@ -73,11 +72,6 @@ module.exports = function( workInProgress.effectTag |= Update; } - function markCallback(workInProgress : Fiber) { - // Tag the fiber with a callback effect. - workInProgress.effectTag |= Callback; - } - function appendAllYields(yields : Array, workInProgress : Fiber) { let node = workInProgress.child; while (node) { @@ -187,26 +181,11 @@ module.exports = function( // Don't use the state queue to compute the memoized state. We already // merged it and assigned it to the instance. Transfer it from there. // Also need to transfer the props, because pendingProps will be null - // in the case of an update + // in the case of an update. const { state, props } = workInProgress.stateNode; - const updateQueue = workInProgress.updateQueue; workInProgress.memoizedState = state; workInProgress.memoizedProps = props; - if (current) { - if (current.memoizedProps !== workInProgress.memoizedProps || - current.memoizedState !== workInProgress.memoizedState || - updateQueue && updateQueue.isForced) { - markUpdate(workInProgress); - } - } else { - markUpdate(workInProgress); - } - if (updateQueue && updateQueue.hasCallback) { - // Transfer update queue to callbackList field so callbacks can be - // called during commit phase. - workInProgress.callbackList = updateQueue; - markCallback(workInProgress); - } + return null; case HostRoot: { workInProgress.memoizedProps = workInProgress.pendingProps; @@ -215,9 +194,6 @@ module.exports = function( fiberRoot.context = fiberRoot.pendingContext; fiberRoot.pendingContext = null; } - // TODO: Only mark this as an update if we have any pending callbacks - // on it. - markUpdate(workInProgress); return null; } case HostComponent: diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 582d7946e3..18c98771ef 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -24,7 +24,7 @@ var { var { createFiberRoot } = require('ReactFiberRoot'); var ReactFiberScheduler = require('ReactFiberScheduler'); -var { createUpdateQueue, addCallbackToQueue } = require('ReactFiberUpdateQueue'); +var { createUpdateQueue, addCallback } = require('ReactFiberUpdateQueue'); if (__DEV__) { var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); @@ -109,16 +109,23 @@ module.exports = function(config : HostConfig, containerInfo : C, parentComponent : ?ReactComponent, callback: ?Function) : OpaqueNode { const context = getContextForSubtree(parentComponent); const root = createFiberRoot(containerInfo, context); - const container = root.current; - if (callback) { - const queue = createUpdateQueue(null); - addCallbackToQueue(queue, callback); - root.callbackList = queue; - } - // TODO: Use pending work/state instead of props. + const current = root.current; + + // TODO: Use the updateQueue and scheduleUpdate, instead of pendingProps. // TODO: This should not override the pendingWorkPriority if there is // higher priority work in the subtree. - container.pendingProps = element; + current.pendingProps = element; + if (current.alternate) { + current.alternate.pendingProps = element; + } + if (callback) { + const queue = current.updateQueue || createUpdateQueue(); + addCallback(queue, callback); + current.updateQueue = queue; + if (current.alternate) { + current.alternate.updateQueue = queue; + } + } scheduleWork(root); @@ -129,24 +136,30 @@ module.exports = function(config : HostConfig, container : OpaqueNode, parentComponent : ?ReactComponent, callback: ?Function) : void { // TODO: If this is a nested container, this won't be the root. const root : FiberRoot = (container.stateNode : any); - if (callback) { - const queue = root.callbackList ? - root.callbackList : - createUpdateQueue(null); - addCallbackToQueue(queue, callback); - root.callbackList = queue; - } + const current = root.current; + root.pendingContext = getContextForSubtree(parentComponent); - // TODO: Use pending work/state instead of props. - root.current.pendingProps = element; - if (root.current.alternate) { - root.current.alternate.pendingProps = element; + + // TODO: Use the updateQueue and scheduleUpdate, instead of pendingProps. + // TODO: This should not override the pendingWorkPriority if there is + // higher priority work in the subtree. + current.pendingProps = element; + if (current.alternate) { + current.alternate.pendingProps = element; + } + if (callback) { + const queue = current.updateQueue || createUpdateQueue(); + addCallback(queue, callback); + current.updateQueue = queue; + if (current.alternate) { + current.alternate.updateQueue = queue; + } } scheduleWork(root); diff --git a/src/renderers/shared/fiber/ReactFiberRoot.js b/src/renderers/shared/fiber/ReactFiberRoot.js index 4da3b9d18e..47ef945dab 100644 --- a/src/renderers/shared/fiber/ReactFiberRoot.js +++ b/src/renderers/shared/fiber/ReactFiberRoot.js @@ -13,7 +13,6 @@ 'use strict'; import type { Fiber } from 'ReactFiber'; -import type { UpdateQueue } from 'ReactFiberUpdateQueue'; const { createHostRootFiber } = require('ReactFiber'); @@ -26,8 +25,6 @@ export type FiberRoot = { isScheduled: boolean, // The work schedule is a linked list. nextScheduledRoot: ?FiberRoot, - // Linked list of callbacks to call after updates are committed. - callbackList: ?UpdateQueue, // Top context object, used by renderSubtreeIntoContainer context: Object, pendingContext: ?Object, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 7bf2f7d7aa..1e858df085 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -392,7 +392,6 @@ module.exports = function(config : HostConfig = + $Subtype | + (prevState: State, props: Props) => $Subtype; + +type Callback = () => void; + +type Update = { + partialState: PartialState, + callback: Callback | null, isReplace: boolean, - next: ?UpdateQueueNode, -}; - -export type UpdateQueue = UpdateQueueNode & { isForced: boolean, - hasUpdate: boolean, + next: Update | null, +}; + +export type UpdateQueue = { + // Points to the first (oldest) update. + first: Update | null, + // Points to the first pending update. A pending update is one that is not + // part of the progressed work. This could be null even in a non-empty queue, + // when none of the updates are empty. + firstPendingUpdate: Update | null, + // Points to the last (newest) update. + last: Update | null, + + // Used to implement forceUpdate. Only true if there's a merged (non-pending) + // force update; pending force updates do not affect this. + hasForceUpdate: boolean, + + // TODO: Remove this by scheduling the side-effect during the begin phase. hasCallback: boolean, - tail: UpdateQueueNode }; -exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue { - const queue = { - partialState, - callback: null, - isReplace: false, - next: null, - isForced: false, - hasUpdate: partialState != null, +exports.createUpdateQueue = function() : UpdateQueue { + return { + first: null, + firstPendingUpdate: null, + last: null, + + hasForceUpdate: false, hasCallback: false, - tail: (null : any), }; - queue.tail = queue; - return queue; }; -function addToQueue(queue : UpdateQueue, partialState : mixed) : UpdateQueue { - const node = { - partialState, - callback: null, - isReplace: false, - next: null, - }; - queue.tail.next = node; - queue.tail = node; - queue.hasUpdate = queue.hasUpdate || (partialState != null); - return queue; +function insertUpdateIntoQueue(queue : UpdateQueue, update : Update) : void { + // Add a pending update to the end of the queue. + // TODO: Once updates have priorities, they should be inserted in the + // correct order. Addtionally, replaceState should remove any pending updates + // that have lower priority from queue. + if (!queue.last) { + // The queue is empty. + queue.first = queue.last = queue.firstPendingUpdate = update; + } else { + // The queue is not empty. Append the update to the end. + queue.last.next = update; + queue.last = update; + + if (!queue.firstPendingUpdate) { + // This is the first pending update. Update the pointer. + queue.firstPendingUpdate = update; + } + } } -exports.addToQueue = addToQueue; - -exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function) : UpdateQueue { - if (queue.tail.callback) { - // If the tail already as a callback, add an empty node to queue - addToQueue(queue, null); - } - queue.tail.callback = callback; - queue.hasCallback = true; - return queue; +exports.addUpdate = function(queue : UpdateQueue, partialState : PartialState | null) : void { + const update = { + partialState, + callback: null, + isReplace: false, + isForced: false, + next: null, + }; + insertUpdateIntoQueue(queue, update); }; -exports.callCallbacks = function(queue : UpdateQueue, context : any) { - let node : ?UpdateQueueNode = queue; - while (node) { - const callback = node.callback; - if (callback) { - if (typeof context !== 'undefined') { - callback.call(context); - } else { - callback(); +exports.addReplaceUpdate = function(queue : UpdateQueue, state : any | null) : void { + const replaceUpdate = { + partialState: state, + callback: null, + isReplace: true, + isForced: false, + next: null, + }; + + + if (!queue.last) { + // The queue is empty. + queue.first = queue.last = queue.firstPendingUpdate = replaceUpdate; + } else { + // The queue is not empty. + + // Drop all existing pending updates. + // TODO: Only drop updates with matching priority. + let lastMergedUpdate = null; + if (queue.firstPendingUpdate) { + let node = queue.first; + while (node && node.next !== queue.firstPendingUpdate) { + node = node.next; + } + lastMergedUpdate = node; + } else { + lastMergedUpdate = queue.last; + } + + if (lastMergedUpdate) { + // Append the new update to the end of the list. + // $FlowFixMe: Union bug (I think? Getting "object literal - This type incompatible with null") + lastMergedUpdate.next = replaceUpdate; + queue.firstPendingUpdate = replaceUpdate; + queue.last = replaceUpdate; + } else { + // Drop everything + queue.first = queue.firstPendingUpdate = queue.last = replaceUpdate; + } + } + +}; + +exports.addForceUpdate = function(queue : UpdateQueue) : void { + const update = { + partialState: null, + callback: null, + isReplace: false, + isForced: true, + next: null, + }; + insertUpdateIntoQueue(queue, update); +}; + + +exports.addCallback = function(queue : UpdateQueue, callback: Callback) : void { + if (queue.firstPendingUpdate && queue.last && !queue.last.callback) { + // If pending updates already exist, and the last pending update does not + // have a callback, we can add the new callback to that update. + // TODO: Add an additional check to ensure the priority matches. + queue.last.callback = callback; + return; + } + + const update = { + partialState: null, + callback, + isReplace: false, + isForced: false, + next: null, + }; + insertUpdateIntoQueue(queue, update); +}; + +exports.hasPendingUpdate = function(queue : UpdateQueue) : boolean { + // TODO: Check priority level + return queue.firstPendingUpdate !== null; +}; + +function getStateFromUpdate(update, instance, prevState, props) { + const partialState = update.partialState; + if (typeof partialState === 'function') { + const updateFn = partialState; + return updateFn.call(instance, prevState, props); + } else { + return partialState; + } +} + +// TODO: Move callback effect scheduling here. Rename to beginUpdateQueue or similar. +exports.mergeQueue = function(queue : UpdateQueue, instance : any, prevState : any, props : any) : any { + // This merges the entire update queue into a single object, not just the + // pending updates, because the previous state and props may have changed. + // TODO: Would memoization be worth it? + + // Reset these flags. We'll update them while looping through the queue. + queue.hasForceUpdate = false; + queue.hasCallback = false; + + let state = prevState; + let dontMutatePrevState = true; + let update : Update | null = queue.first; + let isEmpty = true; + + // TODO: Stop merging once we reach an update whose priority doesn't match. + // Should this also apply to updates that were previous merged but bailed out? + while (update) { + let partialState; + if (update.isReplace) { + // A replace should drop all previous updates in the queue, so + // use the original `prevState`, not the accumulated `state` + state = getStateFromUpdate(update, instance, prevState, props); + dontMutatePrevState = true; + isEmpty = false; + } else { + partialState = getStateFromUpdate(update, instance, state, props); + if (partialState) { + if (dontMutatePrevState) { + state = Object.assign({}, state, partialState); + } else { + state = Object.assign(state, partialState); + } + dontMutatePrevState = false; + isEmpty = false; } } - node = node.next; - } -}; - -function getStateFromNode(node, instance, state, props) { - if (typeof node.partialState === 'function') { - const updateFn = node.partialState; - return updateFn.call(instance, state, props); - } else { - return node.partialState; - } -} - -exports.mergeUpdateQueue = function(queue : UpdateQueue, instance : any, prevState : any, props : any) : any { - let node : ?UpdateQueueNode = queue; - if (queue.isReplace) { - // replaceState is always first in the queue. - prevState = getStateFromNode(queue, instance, prevState, props); - node = queue.next; - if (!node) { - // If there is no more work, we replace the raw object instead of cloning. - return prevState; + if (update.isForced) { + queue.hasForceUpdate = true; } + if (update.callback) { + queue.hasCallback = true; + } + update = update.next; } - let state = Object.assign({}, prevState); - while (node) { - let partialState = getStateFromNode(node, instance, state, props); - Object.assign(state, partialState); - node = node.next; + + // The next pending update is the one that we exited on in the loop above. + // Until priorities are implemented, this is always null. + queue.firstPendingUpdate = update; + + if (isEmpty) { + // None of the updates contained state. Return the original state object. + return prevState; } + return state; }; + +exports.commitUpdateQueue = function(finishedWork : Fiber, queue : UpdateQueue, context : mixed) { + if (queue.hasCallback) { + // Call the callbacks on all the non-pending updates. + let update = queue.first; + while (update && update !== queue.firstPendingUpdate) { + const callback = update.callback; + if (typeof callback === 'function') { + callback.call(context); + } + update = update.next; + } + } + + // Drop all completed updates, leaving only the pending updates. + queue.first = queue.firstPendingUpdate; + if (!queue.first) { + // If the list is now empty, we can remove it from the finished work + finishedWork.updateQueue = null; + if (finishedWork.alternate) { + // Normally we don't mutate the current tree, but we do for updates. + // The queue on the work in progress is always the same as the queue + // on the current. + finishedWork.alternate.updateQueue = null; + } + } +}; From 6301086c354e5c296ed16f1feb9d7eae03444049 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 9 Dec 2016 11:44:31 -0800 Subject: [PATCH 02/16] Schedule callback effects while merging updates This allows us to remove the hasCallback flag. --- .../shared/fiber/ReactFiberBeginWork.js | 14 ++------------ .../shared/fiber/ReactFiberClassComponent.js | 9 +++++---- .../shared/fiber/ReactFiberUpdateQueue.js | 18 +++++++++--------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index fcc89e0532..3c37af0cc1 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -27,7 +27,7 @@ var { } = require('ReactChildFiber'); var { hasPendingUpdate, - mergeQueue, + beginUpdateQueue, } = require('ReactFiberUpdateQueue'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { @@ -57,7 +57,6 @@ var { } = require('ReactPriorityLevel'); var { Update, - Callback, Placement, ContentReset, Err, @@ -217,12 +216,6 @@ module.exports = function( } // Schedule side-effects - const updateQueue = workInProgress.updateQueue; - if (updateQueue && updateQueue.hasCallback) { - // The update queue has a callback. Schedule a callback effect. - // Callbacks are scheduled regardless of whether we bail out below. - workInProgress.effectTag |= Callback; - } if (shouldUpdate) { workInProgress.effectTag |= Update; } else { @@ -537,10 +530,7 @@ module.exports = function( if (updateQueue) { // The last three arguments are unimportant because there should be // no update functions in a HostRoot's queue. - mergeQueue(updateQueue, null, null, null); - if (updateQueue.hasCallback) { - workInProgress.effectTag |= Callback; - } + beginUpdateQueue(workInProgress, updateQueue, null, null, null); } pushHostContainer(workInProgress.stateNode.containerInfo); diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index b72c6bb777..0380a0e19d 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -24,7 +24,7 @@ var { addReplaceUpdate, addForceUpdate, addCallback, - mergeQueue, + beginUpdateQueue, } = require('ReactFiberUpdateQueue'); var { getComponentName, isMounted } = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); @@ -78,6 +78,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState, newContext) { const updateQueue = workInProgress.updateQueue; if (oldProps === null || (updateQueue && updateQueue.hasForceUpdate)) { + // If the workInProgress already has an Update effect, return true return true; } @@ -244,7 +245,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // process them now. const updateQueue = workInProgress.updateQueue; if (updateQueue) { - instance.state = mergeQueue(updateQueue, instance, state, props); + instance.state = beginUpdateQueue(workInProgress, updateQueue, instance, state, props); } } } @@ -293,7 +294,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // during initial mounting. const newUpdateQueue = workInProgress.updateQueue; if (newUpdateQueue) { - newInstance.state = mergeQueue(newUpdateQueue, newInstance, newState, newProps); + newInstance.state = beginUpdateQueue(workInProgress, newUpdateQueue, newInstance, newState, newProps); } return true; } @@ -331,7 +332,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // TODO: Previous state can be null. let newState; if (updateQueue) { - newState = mergeQueue(updateQueue, instance, oldState, newProps); + newState = beginUpdateQueue(workInProgress, updateQueue, instance, oldState, newProps); } else { newState = oldState; } diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index d7c1b00790..8174625a5a 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -14,6 +14,10 @@ import type { Fiber } from 'ReactFiber'; +const { + Callback: CallbackEffect, +} = require('ReactTypeOfSideEffect'); + type PartialState = $Subtype | (prevState: State, props: Props) => $Subtype; @@ -40,10 +44,8 @@ export type UpdateQueue = { // Used to implement forceUpdate. Only true if there's a merged (non-pending) // force update; pending force updates do not affect this. + // TODO: We could use a ForceUpdate effect instead? hasForceUpdate: boolean, - - // TODO: Remove this by scheduling the side-effect during the begin phase. - hasCallback: boolean, }; exports.createUpdateQueue = function() : UpdateQueue { @@ -53,7 +55,6 @@ exports.createUpdateQueue = function() : UpdateQueue { last: null, hasForceUpdate: false, - hasCallback: false, }; }; @@ -177,15 +178,14 @@ function getStateFromUpdate(update, instance, prevState, props) { } } -// TODO: Move callback effect scheduling here. Rename to beginUpdateQueue or similar. -exports.mergeQueue = function(queue : UpdateQueue, instance : any, prevState : any, props : any) : any { +exports.beginUpdateQueue = function(workInProgress : Fiber, queue : UpdateQueue, instance : any, prevState : any, props : any) : any { // This merges the entire update queue into a single object, not just the // pending updates, because the previous state and props may have changed. // TODO: Would memoization be worth it? // Reset these flags. We'll update them while looping through the queue. queue.hasForceUpdate = false; - queue.hasCallback = false; + workInProgress.effectTag |= CallbackEffect; let state = prevState; let dontMutatePrevState = true; @@ -218,7 +218,7 @@ exports.mergeQueue = function(queue : UpdateQueue, instance : any, prevState : a queue.hasForceUpdate = true; } if (update.callback) { - queue.hasCallback = true; + workInProgress.effectTag |= CallbackEffect; } update = update.next; } @@ -236,7 +236,7 @@ exports.mergeQueue = function(queue : UpdateQueue, instance : any, prevState : a }; exports.commitUpdateQueue = function(finishedWork : Fiber, queue : UpdateQueue, context : mixed) { - if (queue.hasCallback) { + if (finishedWork.effectTag & CallbackEffect) { // Call the callbacks on all the non-pending updates. let update = queue.first; while (update && update !== queue.firstPendingUpdate) { From cb1ef03df9019a8235dd1a12539068f030fb48a9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 9 Dec 2016 12:47:18 -0800 Subject: [PATCH 03/16] Replace .hasForceUpdate with ForceUpdate effect Removes another field from the update queue --- .../shared/fiber/ReactFiberBeginWork.js | 2 -- .../shared/fiber/ReactFiberClassComponent.js | 9 +++++---- .../shared/fiber/ReactFiberScheduler.js | 4 +++- .../shared/fiber/ReactFiberUpdateQueue.js | 14 ++++---------- .../shared/fiber/ReactTypeOfSideEffect.js | 19 ++++++++++--------- 5 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 3c37af0cc1..cd818de65c 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -493,8 +493,6 @@ module.exports = function( const memoizedProps = workInProgress.memoizedProps; const updateQueue = workInProgress.updateQueue; - - // This is kept as a single expression to take advantage of short-circuiting. const hasNewProps = ( pendingProps !== null && ( // hasPendingProps && ( diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 0380a0e19d..00886df23b 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -26,6 +26,8 @@ var { addCallback, beginUpdateQueue, } = require('ReactFiberUpdateQueue'); +var { hasContextChanged } = require('ReactFiberContext'); +var { ForceUpdate } = require('ReactTypeOfSideEffect'); var { getComponentName, isMounted } = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); var shallowEqual = require('shallowEqual'); @@ -76,8 +78,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { }; function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState, newContext) { - const updateQueue = workInProgress.updateQueue; - if (oldProps === null || (updateQueue && updateQueue.hasForceUpdate)) { + if (oldProps === null || (workInProgress.effectTag & ForceUpdate)) { // If the workInProgress already has an Update effect, return true return true; } @@ -339,8 +340,8 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { if (oldProps === newProps && oldState === newState && - oldContext === newContext && - updateQueue && !updateQueue.hasForceUpdate) { + !hasContextChanged() && + !(workInProgress.effectTag & ForceUpdate)) { return false; } diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 1e858df085..aa2a392aff 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -40,6 +40,7 @@ var { Placement, Update, PlacementAndUpdate, + ForceUpdate, Deletion, ContentReset, Callback, @@ -199,7 +200,8 @@ module.exports = function(config : HostConfig Date: Fri, 9 Dec 2016 15:06:56 -0800 Subject: [PATCH 04/16] Separate priority for updates When resetting the priority in the complete phase, check the priority of the update queue so that updates aren't dropped. Updates inside render, child cWRP, etc are no longer dropped. The next step is sort the queue by priority and only flush updates that match the current priority level. --- scripts/fiber/tests-failing.txt | 5 - scripts/fiber/tests-passing-except-dev.txt | 2 + scripts/fiber/tests-passing.txt | 1 + .../shared/fiber/ReactFiberBeginWork.js | 25 ++-- .../shared/fiber/ReactFiberClassComponent.js | 80 ++++++++----- .../shared/fiber/ReactFiberReconciler.js | 14 ++- .../shared/fiber/ReactFiberScheduler.js | 45 +++++--- .../shared/fiber/ReactFiberUpdateQueue.js | 108 ++++++++++++++---- 8 files changed, 187 insertions(+), 93 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 034f765d97..da68b422e9 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -71,13 +71,8 @@ src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js * should carry through each of the phases of setup src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js -* should warn about `setState` in render -* should warn about `setState` in getChildContext * should update refs if shouldComponentUpdate gives false -src/renderers/shared/shared/__tests__/ReactCompositeComponentState-test.js -* should update state when called from child cWRP - src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js * should still throw when rendering to undefined * throws when rendering null at the top level diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index 6464c7d7ee..b112148a6e 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -114,6 +114,8 @@ src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js src/renderers/shared/shared/__tests__/ReactCompositeComponent-test.js * should warn about `forceUpdate` on unmounted components * should warn about `setState` on unmounted components +* should warn about `setState` in render +* should warn about `setState` in getChildContext * should disallow nested render calls src/renderers/shared/shared/__tests__/ReactMultiChild-test.js diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 21a5783ffb..74f4310b92 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1382,6 +1382,7 @@ src/renderers/shared/shared/__tests__/ReactCompositeComponentState-test.js * should support setting state * should call componentDidUpdate of children first * should batch unmounts +* should update state when called from child cWRP src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js * should not produce child DOM nodes for null and false diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index cd818de65c..03b36473fd 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -71,7 +71,8 @@ if (__DEV__) { module.exports = function( config : HostConfig, hostContext : HostContext, - scheduleUpdate : (fiber: Fiber) => void + scheduleUpdateAtPriority : (fiber: Fiber, priorityLevel : PriorityLevel) => void, + getPriorityContext : () => PriorityLevel ) { const { shouldSetTextContent } = config; @@ -88,7 +89,7 @@ module.exports = function( mountClassInstance, resumeMountClassInstance, updateClassInstance, - } = ReactFiberClassComponent(scheduleUpdate); + } = ReactFiberClassComponent(scheduleUpdateAtPriority, getPriorityContext); function markChildAsProgressed(current, workInProgress, priorityLevel) { // We now have clones. Let's store them as the currently progressed work. @@ -199,20 +200,20 @@ module.exports = function( return workInProgress.child; } - function updateClassComponent(current : ?Fiber, workInProgress : Fiber) { + function updateClassComponent(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) { let shouldUpdate; if (!current) { if (!workInProgress.stateNode) { // In the initial pass we might need to construct the instance. constructClassInstance(workInProgress); - mountClassInstance(workInProgress); + mountClassInstance(workInProgress, priorityLevel); shouldUpdate = true; } else { // In a resume, we'll already have an instance we can reuse. - shouldUpdate = resumeMountClassInstance(workInProgress); + shouldUpdate = resumeMountClassInstance(workInProgress, priorityLevel); } } else { - shouldUpdate = updateClassInstance(current, workInProgress); + shouldUpdate = updateClassInstance(current, workInProgress, priorityLevel); } // Schedule side-effects @@ -305,7 +306,7 @@ module.exports = function( } } - function mountIndeterminateComponent(current, workInProgress) { + function mountIndeterminateComponent(current, workInProgress, priorityLevel) { if (current) { throw new Error('An indeterminate component should never have mounted.'); } @@ -326,7 +327,7 @@ module.exports = function( // Proceed under the assumption that this is a class instance workInProgress.tag = ClassComponent; adoptClassInstance(workInProgress, value); - mountClassInstance(workInProgress); + mountClassInstance(workInProgress, priorityLevel); ReactCurrentOwner.current = workInProgress; value = value.render(); } else { @@ -509,11 +510,11 @@ module.exports = function( switch (workInProgress.tag) { case IndeterminateComponent: - return mountIndeterminateComponent(current, workInProgress); + return mountIndeterminateComponent(current, workInProgress, priorityLevel); case FunctionalComponent: return updateFunctionalComponent(current, workInProgress); case ClassComponent: - return updateClassComponent(current, workInProgress); + return updateClassComponent(current, workInProgress, priorityLevel); case HostRoot: { const root = (workInProgress.stateNode : FiberRoot); if (root.pendingContext) { @@ -526,9 +527,7 @@ module.exports = function( } if (updateQueue) { - // The last three arguments are unimportant because there should be - // no update functions in a HostRoot's queue. - beginUpdateQueue(workInProgress, updateQueue, null, null, null); + beginUpdateQueue(workInProgress, updateQueue, null, null, null, priorityLevel); } pushHostContainer(workInProgress.stateNode.containerInfo); diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 00886df23b..35f948231b 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -13,13 +13,13 @@ 'use strict'; import type { Fiber } from 'ReactFiber'; -import type { UpdateQueue } from 'ReactFiberUpdateQueue'; +import type { PriorityLevel } from 'ReactPriorityLevel'; var { getMaskedContext, } = require('ReactFiberContext'); var { - createUpdateQueue, + ensureUpdateQueue, addUpdate, addReplaceUpdate, addForceUpdate, @@ -36,44 +36,41 @@ var invariant = require('invariant'); const isArray = Array.isArray; -module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { - - function scheduleUpdateQueue(fiber: Fiber, updateQueue: UpdateQueue) { - fiber.updateQueue = updateQueue; - // Schedule update on the alternate as well, since we don't know which tree - // is current. - if (fiber.alternate) { - fiber.alternate.updateQueue = updateQueue; - } - scheduleUpdate(fiber); - } +module.exports = function( + scheduleUpdateAtPriority : (fiber: Fiber, priorityLevel : PriorityLevel) => void, + getPriorityContext : () => PriorityLevel, +) { // Class component state updater const updater = { isMounted, enqueueSetState(instance, partialState) { const fiber = ReactInstanceMap.get(instance); - const queue = fiber.updateQueue || createUpdateQueue(); - addUpdate(queue, partialState); - scheduleUpdateQueue(fiber, queue); + const queue = ensureUpdateQueue(fiber); + const priorityLevel = getPriorityContext(); + addUpdate(queue, partialState, priorityLevel); + scheduleUpdateAtPriority(fiber, priorityLevel); }, enqueueReplaceState(instance, state) { const fiber = ReactInstanceMap.get(instance); - const queue = fiber.updateQueue || createUpdateQueue(); - addReplaceUpdate(queue, state); - scheduleUpdateQueue(fiber, queue); + const queue = ensureUpdateQueue(fiber); + const priorityLevel = getPriorityContext(); + addReplaceUpdate(queue, state, priorityLevel); + scheduleUpdateAtPriority(fiber, priorityLevel); }, enqueueForceUpdate(instance) { const fiber = ReactInstanceMap.get(instance); - const queue = fiber.updateQueue || createUpdateQueue(); - addForceUpdate(queue); - scheduleUpdateQueue(fiber, queue); + const queue = ensureUpdateQueue(fiber); + const priorityLevel = getPriorityContext(); + addForceUpdate(queue, priorityLevel); + scheduleUpdateAtPriority(fiber, priorityLevel); }, enqueueCallback(instance, callback) { const fiber = ReactInstanceMap.get(instance); - const queue = fiber.updateQueue || createUpdateQueue(); - addCallback(queue, callback); - scheduleUpdateQueue(fiber, queue); + const queue = ensureUpdateQueue(fiber); + const priorityLevel = getPriorityContext(); + addCallback(queue, callback, priorityLevel); + scheduleUpdateAtPriority(fiber, priorityLevel); }, }; @@ -227,7 +224,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { } // Invokes the mount life-cycles on a previously never rendered instance. - function mountClassInstance(workInProgress : Fiber) : void { + function mountClassInstance(workInProgress : Fiber, priorityLevel : PriorityLevel) : void { const instance = workInProgress.stateNode; const state = instance.state || null; @@ -246,14 +243,21 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // process them now. const updateQueue = workInProgress.updateQueue; if (updateQueue) { - instance.state = beginUpdateQueue(workInProgress, updateQueue, instance, state, props); + instance.state = beginUpdateQueue( + workInProgress, + updateQueue, + instance, + state, + props, + priorityLevel + ); } } } // Called on a preexisting class instance. Returns false if a resumed render // could be reused. - function resumeMountClassInstance(workInProgress : Fiber) : boolean { + function resumeMountClassInstance(workInProgress : Fiber, priorityLevel : PriorityLevel) : boolean { let newState = workInProgress.memoizedState; let newProps = workInProgress.pendingProps; if (!newProps) { @@ -295,13 +299,20 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // during initial mounting. const newUpdateQueue = workInProgress.updateQueue; if (newUpdateQueue) { - newInstance.state = beginUpdateQueue(workInProgress, newUpdateQueue, newInstance, newState, newProps); + newInstance.state = beginUpdateQueue( + workInProgress, + newUpdateQueue, + newInstance, + newState, + newProps, + priorityLevel + ); } return true; } // Invokes the update life-cycles and returns false if it shouldn't rerender. - function updateClassInstance(current : Fiber, workInProgress : Fiber) : boolean { + function updateClassInstance(current : Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : boolean { const instance = workInProgress.stateNode; const oldProps = workInProgress.memoizedProps || current.memoizedProps; @@ -333,7 +344,14 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // TODO: Previous state can be null. let newState; if (updateQueue) { - newState = beginUpdateQueue(workInProgress, updateQueue, instance, oldState, newProps); + newState = beginUpdateQueue( + workInProgress, + updateQueue, + instance, + oldState, + newProps, + priorityLevel + ); } else { newState = oldState; } diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 18c98771ef..7eb8208114 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -24,7 +24,7 @@ var { var { createFiberRoot } = require('ReactFiberRoot'); var ReactFiberScheduler = require('ReactFiberScheduler'); -var { createUpdateQueue, addCallback } = require('ReactFiberUpdateQueue'); +var { ensureUpdateQueue, addCallback } = require('ReactFiberUpdateQueue'); if (__DEV__) { var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); @@ -99,6 +99,7 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig = $Subtype | (prevState: State, props: Props) => $Subtype; @@ -26,6 +29,7 @@ type PartialState = type Callback = () => void; type Update = { + priorityLevel: PriorityLevel, partialState: PartialState, callback: Callback | null, isReplace: boolean, @@ -44,19 +48,31 @@ export type UpdateQueue = { last: Update | null, }; -exports.createUpdateQueue = function() : UpdateQueue { - return { +// Ensures that a fiber and its alternate have an update queue, creating a newText +// one if needed. Returns the new or existing queue. +function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { + if (fiber.updateQueue) { + // We already have an update queue. + return fiber.updateQueue; + } + const queue = { first: null, firstPendingUpdate: null, last: null, }; -}; + fiber.updateQueue = queue; + // Add queue to the alternate as well, because when we call setState we don't + // know which tree is current. + if (fiber.alternate) { + fiber.alternate.updateQueue = queue; + } + return queue; +} +exports.ensureUpdateQueue = ensureUpdateQueue; -function insertUpdateIntoQueue(queue : UpdateQueue, update : Update) : void { +function insertUpdateIntoQueue(queue : UpdateQueue, update : Update, priorityLevel : PriorityLevel) : void { // Add a pending update to the end of the queue. - // TODO: Once updates have priorities, they should be inserted in the - // correct order. Addtionally, replaceState should remove any pending updates - // that have lower priority from queue. + // TODO: Insert updates in order of priority. if (!queue.last) { // The queue is empty. queue.first = queue.last = queue.firstPendingUpdate = update; @@ -72,19 +88,30 @@ function insertUpdateIntoQueue(queue : UpdateQueue, update : Update) : void { } } -exports.addUpdate = function(queue : UpdateQueue, partialState : PartialState | null) : void { +function addUpdate( + queue : UpdateQueue, + partialState : PartialState | null, + priorityLevel : PriorityLevel +) : void { const update = { + priorityLevel, partialState, callback: null, isReplace: false, isForced: false, next: null, }; - insertUpdateIntoQueue(queue, update); -}; + insertUpdateIntoQueue(queue, update, priorityLevel); +} +exports.addUpdate = addUpdate; -exports.addReplaceUpdate = function(queue : UpdateQueue, state : any | null) : void { +function addReplaceUpdate( + queue : UpdateQueue, + state : any | null, + priorityLevel : PriorityLevel +) : void { const replaceUpdate = { + priorityLevel, partialState: state, callback: null, isReplace: true, @@ -92,7 +119,6 @@ exports.addReplaceUpdate = function(queue : UpdateQueue, state : any | null) : v next: null, }; - if (!queue.last) { // The queue is empty. queue.first = queue.last = queue.firstPendingUpdate = replaceUpdate; @@ -124,21 +150,24 @@ exports.addReplaceUpdate = function(queue : UpdateQueue, state : any | null) : v } } -}; +} +exports.addReplaceUpdate = addReplaceUpdate; -exports.addForceUpdate = function(queue : UpdateQueue) : void { +function addForceUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : void { const update = { + priorityLevel, partialState: null, callback: null, isReplace: false, isForced: true, next: null, }; - insertUpdateIntoQueue(queue, update); -}; + insertUpdateIntoQueue(queue, update, priorityLevel); +} +exports.addForceUpdate = addForceUpdate; -exports.addCallback = function(queue : UpdateQueue, callback: Callback) : void { +function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : PriorityLevel) : void { if (queue.firstPendingUpdate && queue.last && !queue.last.callback) { // If pending updates already exist, and the last pending update does not // have a callback, we can add the new callback to that update. @@ -148,19 +177,39 @@ exports.addCallback = function(queue : UpdateQueue, callback: Callback) : void { } const update = { + priorityLevel, partialState: null, callback, isReplace: false, isForced: false, next: null, }; - insertUpdateIntoQueue(queue, update); -}; + insertUpdateIntoQueue(queue, update, priorityLevel); +} +exports.addCallback = addCallback; -exports.hasPendingUpdate = function(queue : UpdateQueue) : boolean { +function hasPendingUpdate(queue : UpdateQueue) : boolean { // TODO: Check priority level return queue.firstPendingUpdate !== null; -}; +} +exports.hasPendingUpdate = hasPendingUpdate; + +function getPendingPriority(queue : UpdateQueue) : PriorityLevel { + // Loop through the pending updates to recompute the pending priority. + // TODO: Once updates are sorted, just read from the first pending update. + let priorityLevel = NoWork; + let update = queue.firstPendingUpdate; + while (update) { + if (priorityLevel === NoWork || + priorityLevel >= update.priorityLevel) { + // Update pending priority + priorityLevel = update.priorityLevel; + } + update = update.next; + } + return priorityLevel; +} +exports.getPendingPriority = getPendingPriority; function getStateFromUpdate(update, instance, prevState, props) { const partialState = update.partialState; @@ -172,7 +221,14 @@ function getStateFromUpdate(update, instance, prevState, props) { } } -exports.beginUpdateQueue = function(workInProgress : Fiber, queue : UpdateQueue, instance : any, prevState : any, props : any) : any { +function beginUpdateQueue( + workInProgress : Fiber, + queue : UpdateQueue, + instance : any, + prevState : any, + props : any, + priorityLevel : PriorityLevel +) : any { // This merges the entire update queue into a single object, not just the // pending updates, because the previous state and props may have changed. // TODO: Would memoization be worth it? @@ -227,9 +283,10 @@ exports.beginUpdateQueue = function(workInProgress : Fiber, queue : UpdateQueue, } return state; -}; +} +exports.beginUpdateQueue = beginUpdateQueue; -exports.commitUpdateQueue = function(finishedWork : Fiber, queue : UpdateQueue, context : mixed) { +function commitUpdateQueue(finishedWork : Fiber, queue : UpdateQueue, context : mixed) { if (finishedWork.effectTag & CallbackEffect) { // Call the callbacks on all the non-pending updates. let update = queue.first; @@ -254,4 +311,5 @@ exports.commitUpdateQueue = function(finishedWork : Fiber, queue : UpdateQueue, finishedWork.alternate.updateQueue = null; } } -}; +} +exports.commitUpdateQueue = commitUpdateQueue; From bb844a03b2c1c1910baefeb2e24bc29d6095afd5 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 10 Dec 2016 02:24:49 -0800 Subject: [PATCH 05/16] Use lastProgressedUpdate pointer instead of firstPendingUpdate We need to be able to access both, and since the list uses forward pointers, it makes more sense to point to the one that comes first. Otherwise to get the last progressed update you have to start at the beginning of the list. --- src/renderers/noop/ReactNoop.js | 8 +- .../shared/fiber/ReactFiberBeginWork.js | 2 +- .../shared/fiber/ReactFiberUpdateQueue.js | 91 +++++++++---------- 3 files changed, 53 insertions(+), 48 deletions(-) diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index c3ca7bc633..8804b14f3e 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -286,10 +286,16 @@ var ReactNoop = { log( ' '.repeat(depth + 1) + 'QUEUED UPDATES' ); - const firstPendingUpdate = updateQueue.firstPendingUpdate; + let firstPendingUpdate; + if (updateQueue.lastProgressedUpdate) { + firstPendingUpdate = updateQueue.lastProgressedUpdate.next; + } else { + firstPendingUpdate = updateQueue.first; + } if (!firstPendingUpdate) { return; } + log( ' '.repeat(depth + 1) + '~', firstPendingUpdate && firstPendingUpdate.partialState, diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 03b36473fd..f00bcca831 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -502,7 +502,7 @@ module.exports = function( ) // ) ); if (!hasNewProps) { - const hasUpdate = updateQueue && hasPendingUpdate(updateQueue); + const hasUpdate = updateQueue && hasPendingUpdate(updateQueue, priorityLevel); if (!hasUpdate && !hasContextChanged()) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 7083979136..e823abe8ca 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -38,17 +38,34 @@ type Update = { }; export type UpdateQueue = { - // Points to the first (oldest) update. + // Points to the first update. first: Update | null, - // Points to the first pending update. A pending update is one that is not - // part of the progressed work. This could be null even in a non-empty queue, - // when none of the updates are empty. - firstPendingUpdate: Update | null, - // Points to the last (newest) update. + lastProgressedUpdate: Update | null, + // Points to the last update. last: Update | null, }; -// Ensures that a fiber and its alternate have an update queue, creating a newText +function getFirstPendingUpdate(queue : UpdateQueue) { + if (queue.lastProgressedUpdate) { + return queue.lastProgressedUpdate.next; + } + return queue.first; +} + +function getFirstProgressedUpdate(queue : UpdateQueue) { + if (queue.lastProgressedUpdate) { + return queue.first; + } + return null; +} + +function hasPendingUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : boolean { + // TODO: Check priority level + return Boolean(getFirstPendingUpdate(queue)); +} +exports.hasPendingUpdate = hasPendingUpdate; + +// Ensures that a fiber and its alternate have an update queue, creating a new // one if needed. Returns the new or existing queue. function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { if (fiber.updateQueue) { @@ -57,7 +74,7 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { } const queue = { first: null, - firstPendingUpdate: null, + lastProgressedUpdate: null, last: null, }; fiber.updateQueue = queue; @@ -75,15 +92,13 @@ function insertUpdateIntoQueue(queue : UpdateQueue, update : Update, priorityLev // TODO: Insert updates in order of priority. if (!queue.last) { // The queue is empty. - queue.first = queue.last = queue.firstPendingUpdate = update; + queue.first = queue.last = update; } else { // The queue is not empty. Append the update to the end. queue.last.next = update; queue.last = update; - - if (!queue.firstPendingUpdate) { - // This is the first pending update. Update the pointer. - queue.firstPendingUpdate = update; + if (queue.lastProgressedUpdate && !queue.lastProgressedUpdate.next) { + queue.lastProgressedUpdate.next = update; } } } @@ -121,32 +136,18 @@ function addReplaceUpdate( if (!queue.last) { // The queue is empty. - queue.first = queue.last = queue.firstPendingUpdate = replaceUpdate; + queue.first = queue.last = replaceUpdate; } else { // The queue is not empty. // Drop all existing pending updates. // TODO: Only drop updates with matching priority. - let lastMergedUpdate = null; - if (queue.firstPendingUpdate) { - let node = queue.first; - while (node && node.next !== queue.firstPendingUpdate) { - node = node.next; - } - lastMergedUpdate = node; - } else { - lastMergedUpdate = queue.last; - } - - if (lastMergedUpdate) { - // Append the new update to the end of the list. - // $FlowFixMe: Union bug (I think? Getting "object literal - This type incompatible with null") - lastMergedUpdate.next = replaceUpdate; - queue.firstPendingUpdate = replaceUpdate; + if (queue.lastProgressedUpdate) { + queue.lastProgressedUpdate.next = replaceUpdate; queue.last = replaceUpdate; } else { // Drop everything - queue.first = queue.firstPendingUpdate = queue.last = replaceUpdate; + queue.first = queue.last = replaceUpdate; } } @@ -168,7 +169,7 @@ exports.addForceUpdate = addForceUpdate; function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : PriorityLevel) : void { - if (queue.firstPendingUpdate && queue.last && !queue.last.callback) { + if (getFirstPendingUpdate(queue) && queue.last && !queue.last.callback) { // If pending updates already exist, and the last pending update does not // have a callback, we can add the new callback to that update. // TODO: Add an additional check to ensure the priority matches. @@ -188,17 +189,12 @@ function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : Pr } exports.addCallback = addCallback; -function hasPendingUpdate(queue : UpdateQueue) : boolean { - // TODO: Check priority level - return queue.firstPendingUpdate !== null; -} -exports.hasPendingUpdate = hasPendingUpdate; - function getPendingPriority(queue : UpdateQueue) : PriorityLevel { // Loop through the pending updates to recompute the pending priority. // TODO: Once updates are sorted, just read from the first pending update. let priorityLevel = NoWork; - let update = queue.firstPendingUpdate; + // Start with first pending update + let update = getFirstPendingUpdate(queue); while (update) { if (priorityLevel === NoWork || priorityLevel >= update.priorityLevel) { @@ -239,11 +235,12 @@ function beginUpdateQueue( let state = prevState; let dontMutatePrevState = true; - let update : Update | null = queue.first; let isEmpty = true; // TODO: Stop merging once we reach an update whose priority doesn't match. // Should this also apply to updates that were previous merged but bailed out? + let update : Update | null = queue.first; + let lastProgressedUpdate = null; while (update) { let partialState; if (update.isReplace) { @@ -270,12 +267,11 @@ function beginUpdateQueue( if (update.callback) { workInProgress.effectTag |= CallbackEffect; } + lastProgressedUpdate = update; update = update.next; } - // The next pending update is the one that we exited on in the loop above. - // Until priorities are implemented, this is always null. - queue.firstPendingUpdate = update; + queue.lastProgressedUpdate = lastProgressedUpdate; if (isEmpty) { // None of the updates contained state. Return the original state object. @@ -287,10 +283,11 @@ function beginUpdateQueue( exports.beginUpdateQueue = beginUpdateQueue; function commitUpdateQueue(finishedWork : Fiber, queue : UpdateQueue, context : mixed) { + if (finishedWork.effectTag & CallbackEffect) { // Call the callbacks on all the non-pending updates. - let update = queue.first; - while (update && update !== queue.firstPendingUpdate) { + let update = getFirstProgressedUpdate(queue); + while (update && update !== getFirstPendingUpdate(queue)) { const callback = update.callback; if (typeof callback === 'function') { callback.call(context); @@ -300,8 +297,10 @@ function commitUpdateQueue(finishedWork : Fiber, queue : UpdateQueue, context : } // Drop all completed updates, leaving only the pending updates. - queue.first = queue.firstPendingUpdate; + queue.first = getFirstPendingUpdate(queue); if (!queue.first) { + queue.last = queue.lastProgressedUpdate = null; + // If the list is now empty, we can remove it from the finished work finishedWork.updateQueue = null; if (finishedWork.alternate) { From 20f00045d323475251fec8d9f581d9ce6dc3fe91 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sun, 11 Dec 2016 13:13:54 -0800 Subject: [PATCH 06/16] Apply pending updates in order of priority The queue maintains a pointer to the last progressed update in the list. Updates that come after that pointer are pending. The pointer is set to the end of the list during reconciliation. Pending updates are sorted by priority then insertion. Progressed updates are sorted by the order in which they were applied during reconciliation, which may not be by priority: if a component bails out before the updates are committed, in the next render, the progressed updates are applied in the same order that they were previously, even if a higher priority update comes in. Once a progressed update is flushed/committed, it's removed from the queue. --- scripts/fiber/tests-passing.txt | 8 + .../shared/fiber/ReactFiberUpdateQueue.js | 229 +++++++++++----- .../__tests__/ReactIncrementalUpdates-test.js | 248 ++++++++++++++++++ 3 files changed, 426 insertions(+), 59 deletions(-) create mode 100644 src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 74f4310b92..ebe26c9d9d 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1230,6 +1230,14 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js * invokes ref callbacks after insertion/update/unmount * supports string refs +src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js +* applies updates in order of priority +* applies updates with equal priority in insertion order +* only drops updates with equal or lesser priority when replaceState is called +* can abort an update, schedule additional updates, and resume +* can abort an update, schedule a replaceState, and resume +* does not call callbacks that are scheduled by another callback until a later commit + src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js * should render a simple fragment at the top of a component * should preserve state when switching from a single child diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index e823abe8ca..e83b0cca93 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -20,7 +20,11 @@ const { Callback: CallbackEffect, } = require('ReactTypeOfSideEffect'); -const { NoWork } = require('ReactPriorityLevel'); +const { + NoWork, + SynchronousPriority, + TaskPriority, +} = require('ReactPriorityLevel'); type PartialState = $Subtype | @@ -37,22 +41,57 @@ type Update = { next: Update | null, }; +// Linked-list of updates +// +// - A "pending" update is one that has been scheduled but not yet used +// for reconciliation. +// - A "progressed" update is an update that was used for reconciliation, but +// has not yet been flushed. +// +// The queue maintains a pointer to the last progressed update in the list. +// Updates that come after that pointer are pending. The pointer is set to the +// end of the list during reconciliation. +// +// Pending updates are sorted by priority then insertion. Progressed updates +// are sorted by the order in which they were applied during reconciliation, +// which may not be by priority: if a component bails out before the updates are +// committed, in the next render, the progressed updates are applied in the same +// order that they were previously, even if a higher priority update comes in. +// +// Once a progressed update is flushed/committed, it's removed from the queue. export type UpdateQueue = { - // Points to the first update. first: Update | null, + // A pointer to the last progressed update in the queue. This may be null + // even in a non-empty queue, if all the updates are pending. lastProgressedUpdate: Update | null, - // Points to the last update. last: Update | null, }; -function getFirstPendingUpdate(queue : UpdateQueue) { +function comparePriority(a : PriorityLevel, b : PriorityLevel) : number { + // When comparing update priorities, treat sync and Task work as equal. + // TODO: Could we avoid the need for this by always coercing sync priority + // to Task when scheduling an update? + if ((a === TaskPriority || a === SynchronousPriority) && + (b === TaskPriority || b === SynchronousPriority)) { + return 0; + } + if (a === NoWork && b !== NoWork) { + return -Infinity; + } + if (a !== NoWork && b === NoWork) { + return Infinity; + } + return a - b; +} + +function getFirstPendingUpdate(queue : UpdateQueue) : Update | null { if (queue.lastProgressedUpdate) { return queue.lastProgressedUpdate.next; } return queue.first; } -function getFirstProgressedUpdate(queue : UpdateQueue) { +function getFirstProgressedUpdate(queue : UpdateQueue) : Update | null { if (queue.lastProgressedUpdate) { return queue.first; } @@ -60,8 +99,12 @@ function getFirstProgressedUpdate(queue : UpdateQueue) { } function hasPendingUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : boolean { - // TODO: Check priority level - return Boolean(getFirstPendingUpdate(queue)); + const firstPendingUpdate = getFirstPendingUpdate(queue); + if (!firstPendingUpdate) { + return false; + } + // Return true if the first pending update has greater or equal priority. + return comparePriority(firstPendingUpdate.priorityLevel, priorityLevel) <= 0; } exports.hasPendingUpdate = hasPendingUpdate; @@ -72,7 +115,7 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { // We already have an update queue. return fiber.updateQueue; } - const queue = { + const queue : UpdateQueue = { first: null, lastProgressedUpdate: null, last: null, @@ -87,20 +130,61 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { } exports.ensureUpdateQueue = ensureUpdateQueue; -function insertUpdateIntoQueue(queue : UpdateQueue, update : Update, priorityLevel : PriorityLevel) : void { +function insertUpdateIntoQueue(queue : UpdateQueue, update : Update) : void { // Add a pending update to the end of the queue. - // TODO: Insert updates in order of priority. if (!queue.last) { // The queue is empty. queue.first = queue.last = update; - } else { - // The queue is not empty. Append the update to the end. + return; + } + // The queue is not empty. Insert the new update into the queue, sorted by + // priority then insertion order. + const firstPendingUpdate = getFirstPendingUpdate(queue); + if (!firstPendingUpdate && queue.last) { + // This is the first pending update. Add it to the end of the queue. queue.last.next = update; queue.last = update; - if (queue.lastProgressedUpdate && !queue.lastProgressedUpdate.next) { - queue.lastProgressedUpdate.next = update; + return; + } + + const priorityLevel = update.priorityLevel; + const lastPendingUpdate = queue.last; + + let insertAfter; + let insertBefore; + if (queue.last && comparePriority(queue.last.priorityLevel, priorityLevel) <= 0) { + // Fast path where the incoming update has equal or lower priority than the + // last pending update. We can just append it to the end of the queue. + insertAfter = lastPendingUpdate; + insertBefore = null; + } else { + // Loop through the pending updates to find the first one with lower + // priority than the incoming update. Insert the incoming update before + // that one. + insertAfter = queue.lastProgressedUpdate; + insertBefore = firstPendingUpdate; + while ( + insertBefore && + comparePriority(insertBefore.priorityLevel, priorityLevel) <= 0 + ) { + insertAfter = insertBefore; + insertBefore = insertBefore.next; } } + + if (insertAfter) { + insertAfter.next = update; + } else { + // This is the first item in the queue. + queue.first = update; + } + + if (insertBefore) { + update.next = insertBefore; + } else { + // This is the last item in the queue. + queue.last = update; + } } function addUpdate( @@ -108,7 +192,7 @@ function addUpdate( partialState : PartialState | null, priorityLevel : PriorityLevel ) : void { - const update = { + const update : Update = { priorityLevel, partialState, callback: null, @@ -116,7 +200,7 @@ function addUpdate( isForced: false, next: null, }; - insertUpdateIntoQueue(queue, update, priorityLevel); + insertUpdateIntoQueue(queue, update); } exports.addUpdate = addUpdate; @@ -125,7 +209,7 @@ function addReplaceUpdate( state : any | null, priorityLevel : PriorityLevel ) : void { - const replaceUpdate = { + const update : Update = { priorityLevel, partialState: state, callback: null, @@ -133,29 +217,57 @@ function addReplaceUpdate( isForced: false, next: null, }; - + // Add a pending update to the end of the queue. if (!queue.last) { // The queue is empty. - queue.first = queue.last = replaceUpdate; - } else { - // The queue is not empty. + queue.first = queue.last = update; + return; + } + // The queue is not empty. Insert the new update into the queue, sorted by + // priority then insertion order. Since this is a replace, drop all pending + // updates with equal priority. We can't drop updates with higher priority, + // because they might be flushed in an earlier commit. We'll drop them during + // the commit phase if necessary. + const firstPendingUpdate = getFirstPendingUpdate(queue); + if (!firstPendingUpdate && queue.last) { + // This is the first pending update. Add it to the end of the queue. + queue.last.next = update; + queue.last = update; + return; + } - // Drop all existing pending updates. - // TODO: Only drop updates with matching priority. - if (queue.lastProgressedUpdate) { - queue.lastProgressedUpdate.next = replaceUpdate; - queue.last = replaceUpdate; - } else { - // Drop everything - queue.first = queue.last = replaceUpdate; + // Find the last pending update with equal priority. + let replaceAfter = queue.lastProgressedUpdate; + let replaceBefore = firstPendingUpdate; + if (replaceBefore) { + let comparison = Infinity; + while (replaceBefore && + (comparison = comparePriority(replaceBefore.priorityLevel, priorityLevel)) <= 0) { + if (comparison < 0) { + replaceAfter = replaceBefore; + } + replaceBefore = replaceBefore.next; } } + if (replaceAfter) { + replaceAfter.next = update; + } else { + // This is the first item in the queue. + queue.first = update; + } + + if (replaceBefore) { + update.next = replaceBefore; + } else { + // This is the last item in the queue. + queue.last = update; + } } exports.addReplaceUpdate = addReplaceUpdate; function addForceUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : void { - const update = { + const update : Update = { priorityLevel, partialState: null, callback: null, @@ -163,21 +275,24 @@ function addForceUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : vo isForced: true, next: null, }; - insertUpdateIntoQueue(queue, update, priorityLevel); + insertUpdateIntoQueue(queue, update); } exports.addForceUpdate = addForceUpdate; function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : PriorityLevel) : void { - if (getFirstPendingUpdate(queue) && queue.last && !queue.last.callback) { + if (getFirstPendingUpdate(queue) && + queue.last && + (queue.last.priorityLevel === priorityLevel) && + !queue.last.callback) { // If pending updates already exist, and the last pending update does not - // have a callback, we can add the new callback to that update. - // TODO: Add an additional check to ensure the priority matches. + // have a callback, and the priority levels are equal, we can add the + // incoming callback to that update to avoid an extra allocation. queue.last.callback = callback; return; } - const update = { + const update : Update = { priorityLevel, partialState: null, callback, @@ -185,25 +300,13 @@ function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : Pr isForced: false, next: null, }; - insertUpdateIntoQueue(queue, update, priorityLevel); + insertUpdateIntoQueue(queue, update); } exports.addCallback = addCallback; function getPendingPriority(queue : UpdateQueue) : PriorityLevel { - // Loop through the pending updates to recompute the pending priority. - // TODO: Once updates are sorted, just read from the first pending update. - let priorityLevel = NoWork; - // Start with first pending update - let update = getFirstPendingUpdate(queue); - while (update) { - if (priorityLevel === NoWork || - priorityLevel >= update.priorityLevel) { - // Update pending priority - priorityLevel = update.priorityLevel; - } - update = update.next; - } - return priorityLevel; + const firstPendingUpdate = getFirstPendingUpdate(queue); + return firstPendingUpdate ? firstPendingUpdate.priorityLevel : NoWork; } exports.getPendingPriority = getPendingPriority; @@ -225,23 +328,25 @@ function beginUpdateQueue( props : any, priorityLevel : PriorityLevel ) : any { - // This merges the entire update queue into a single object, not just the - // pending updates, because the previous state and props may have changed. - // TODO: Would memoization be worth it? + // Applies updates with matching priority to the previous state to create + // a new state object. If an update was used previously but never flushed + // due to a bail out, it's used again regardless of its priority. // Reset these flags. We'll update them while looping through the queue. workInProgress.effectTag &= ~ForceUpdate; workInProgress.effectTag &= ~CallbackEffect; + const prevLastProgressedUpdate = queue.lastProgressedUpdate; let state = prevState; let dontMutatePrevState = true; let isEmpty = true; - - // TODO: Stop merging once we reach an update whose priority doesn't match. - // Should this also apply to updates that were previous merged but bailed out? - let update : Update | null = queue.first; + let alreadyProgressedUpdate = Boolean(prevLastProgressedUpdate); let lastProgressedUpdate = null; - while (update) { + let update : Update | null = queue.first; + while (update && ( + alreadyProgressedUpdate || + comparePriority(update.priorityLevel, priorityLevel) <= 0 + )) { let partialState; if (update.isReplace) { // A replace should drop all previous updates in the queue, so @@ -267,10 +372,15 @@ function beginUpdateQueue( if (update.callback) { workInProgress.effectTag |= CallbackEffect; } + if (update === prevLastProgressedUpdate) { + alreadyProgressedUpdate = false; + } lastProgressedUpdate = update; update = update.next; } + + // Mark the point in the queue where we stopped applying updates queue.lastProgressedUpdate = lastProgressedUpdate; if (isEmpty) { @@ -296,8 +406,9 @@ function commitUpdateQueue(finishedWork : Fiber, queue : UpdateQueue, context : } } - // Drop all completed updates, leaving only the pending updates. + // Drop all comitted updates, leaving only the pending updates. queue.first = getFirstPendingUpdate(queue); + queue.lastProgressedUpdate = null; if (!queue.first) { queue.last = queue.lastProgressedUpdate = null; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js new file mode 100644 index 0000000000..7dbf05ad52 --- /dev/null +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js @@ -0,0 +1,248 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +var React; +var ReactNoop; + +describe('ReactIncrementalUpdates', () => { + beforeEach(() => { + jest.resetModuleRegistry(); + React = require('React'); + ReactNoop = require('ReactNoop'); + }); + + function div(...children) { + children = children.map(c => typeof c === 'string' ? { text: c } : c); + return { type: 'div', children, prop: undefined }; + } + + function span(prop) { + return { type: 'span', children: [], prop }; + } + + it('applies updates in order of priority', () => { + let state; + class Foo extends React.Component { + state = {}; + componentDidMount() { + ReactNoop.performAnimationWork(() => { + // Has Animation priority + this.setState({ b: 'b' }); + this.setState({ c: 'c' }); + }); + // Has Task priority + this.setState({ a: 'a' }); + } + render() { + state = this.state; + return
; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(Object.keys(state)).toEqual(['a', 'b', 'c']); + }); + + it('applies updates with equal priority in insertion order', () => { + let state; + class Foo extends React.Component { + state = {}; + componentDidMount() { + // All have Task priority + this.setState({ a: 'a' }); + this.setState({ b: 'b' }); + this.setState({ c: 'c' }); + } + render() { + state = this.state; + return
; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(Object.keys(state)).toEqual(['a', 'b', 'c']); + }); + + it('only drops updates with equal or lesser priority when replaceState is called', () => { + let instance; + const Foo = React.createClass({ + getInitialState() { + return {}; + }, + render() { + instance = this; + return ( + + ); + }, + }); + + ReactNoop.render(); + ReactNoop.flush(); + + instance.setState({ x: 'x' }); + instance.setState({ y: 'y' }); + ReactNoop.performAnimationWork(() => { + instance.setState({ a: 'a' }); + instance.setState({ b: 'b' }); + }); + instance.replaceState({ c: 'c' }); + instance.setState({ d: 'd' }); + + ReactNoop.flushAnimationPri(); + // Even though a replaceState has been already scheduled, it hasn't been + // flushed yet because it has low priority. + expect(ReactNoop.getChildren()).toEqual([span('ab')]); + + ReactNoop.flush(); + // Now the rest of the updates are flushed. + expect(ReactNoop.getChildren()).toEqual([span('cd')]); + }); + + it('can abort an update, schedule additional updates, and resume', () => { + let instance; + class Foo extends React.Component { + state = {}; + render() { + instance = this; + return ( + + ); + } + } + + ReactNoop.render(); + ReactNoop.flush(); + + let progressedUpdates = []; + function createUpdate(letter) { + return () => { + progressedUpdates.push(letter); + return { + [letter]: letter, + }; + }; + } + + instance.setState(createUpdate('a')); + instance.setState(createUpdate('b')); + instance.setState(createUpdate('c')); + + // Do just enough work to begin the update but not enough to flush it + ReactNoop.flushDeferredPri(20); + expect(ReactNoop.getChildren()).toEqual([span('')]); + expect(progressedUpdates).toEqual(['a', 'b', 'c']); + + instance.setState(createUpdate('f')); + ReactNoop.performAnimationWork(() => { + instance.setState(createUpdate('d')); + instance.setState(createUpdate('e')); + }); + instance.setState(createUpdate('g')); + + // Updates a, b, and c were aborted, so they should be applied first even + // though they have low priority. Update f was scheduled after the render + // was aborted, so it should come after d and e, which have higher priority. + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]); + expect(progressedUpdates).toEqual(['a', 'b', 'c', 'd', 'e', 'f', 'g']); + }); + + it('can abort an update, schedule a replaceState, and resume', () => { + let instance; + const Foo = React.createClass({ + getInitialState() { + return {}; + }, + render() { + instance = this; + return ( + + ); + }, + }); + + ReactNoop.render(); + ReactNoop.flush(); + + let progressedUpdates = []; + function createUpdate(letter) { + return () => { + progressedUpdates.push(letter); + return { + [letter]: letter, + }; + }; + } + + instance.setState(createUpdate('a')); + instance.setState(createUpdate('b')); + instance.setState(createUpdate('c')); + + // Do just enough work to begin the update but not enough to flush it + ReactNoop.flushDeferredPri(20); + expect(ReactNoop.getChildren()).toEqual([span('')]); + expect(progressedUpdates).toEqual(['a', 'b', 'c']); + + progressedUpdates = []; + + instance.setState(createUpdate('f')); + ReactNoop.performAnimationWork(() => { + instance.setState(createUpdate('d')); + instance.replaceState(createUpdate('e')); + }); + instance.setState(createUpdate('g')); + + // Updates a, b, and c were aborted, so they should be applied first even + // though they have low priority. Update f was scheduled after the render + // was aborted, so it should come after d and e, which have higher priority. + // Because e is a replaceState, d gets dropped. + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('efg')]); + // Ensure that update d is not progressed before being replaced. + expect(progressedUpdates).toEqual(['e', 'f', 'g']); + }); + + it('does not call callbacks that are scheduled by another callback until a later commit', () => { + let ops = []; + class Foo extends React.Component { + state = {}; + componentDidMount() { + ops.push('did mount'); + this.setState({ a: 'a' }, () => { + ops.push('callback a'); + this.setState({ b: 'b' }, () => { + ops.push('callback b'); + }); + }); + } + render() { + ops.push('render'); + return
; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual([ + 'render', + 'did mount', + 'render', + 'callback a', + 'render', + 'callback b', + ]); + }); +}); From ead8ab7e2d2a5d318c580675503de761a49745cb Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 12 Dec 2016 13:32:00 -0800 Subject: [PATCH 07/16] Add unstable_deferredUpdates This is needed to get the triangle demo working. --- examples/fiber/index.html | 156 ++++++++++++++++-- src/renderers/dom/fiber/ReactDOMFiber.js | 2 + .../shared/fiber/ReactFiberReconciler.js | 4 + .../shared/fiber/ReactFiberScheduler.js | 11 ++ 4 files changed, 160 insertions(+), 13 deletions(-) diff --git a/examples/fiber/index.html b/examples/fiber/index.html index c90e7ce246..44df6da417 100644 --- a/examples/fiber/index.html +++ b/examples/fiber/index.html @@ -1,5 +1,5 @@ - + Fiber Example @@ -19,26 +19,156 @@
- + diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 4c38e66f6b..19c29643f3 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -242,6 +242,8 @@ var ReactDOM = { unstable_batchedUpdates: ReactGenericBatching.batchedUpdates, + unstable_deferredUpdates: DOMRenderer.deferredUpdates, + }; module.exports = ReactDOM; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 7eb8208114..3547470ab0 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -79,6 +79,7 @@ export type Reconciler = { // FIXME: ESLint complains about type parameter batchedUpdates(fn : () => A) : A, syncUpdates(fn : () => A) : A, + deferredUpdates(fn : () => A) : A, /* eslint-enable no-undef */ // Used to extract the return value from the initial render. Legacy API. @@ -103,6 +104,7 @@ module.exports = function(config : HostConfig(config : HostConfig | I | TI | null) { const root : FiberRoot = (container.stateNode : any); const containerFiber = root.current; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 55bca2d3ba..e002574c34 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -1076,11 +1076,22 @@ module.exports = function(config : HostConfig(fn : () => A) : A { + const previousPriorityContext = priorityContext; + priorityContext = LowPriority; + try { + return fn(); + } finally { + priorityContext = previousPriorityContext; + } + } + return { scheduleWork: scheduleWork, getPriorityContext: getPriorityContext, performWithPriority: performWithPriority, batchedUpdates: batchedUpdates, syncUpdates: syncUpdates, + deferredUpdates: deferredUpdates, }; }; From d7f89b86818c267083e9841a830031438e0d0185 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 12 Dec 2016 23:53:14 -0800 Subject: [PATCH 08/16] Don't rely on commit phase effect to clear updates Instead clear updates on the work-in-progress during the begin phase. Aborted updates are recovered by cloning from the current fiber. --- examples/fiber/index.html | 4 +- src/renderers/noop/ReactNoop.js | 15 +- src/renderers/shared/fiber/ReactFiber.js | 9 +- .../shared/fiber/ReactFiberClassComponent.js | 13 +- .../shared/fiber/ReactFiberCommitWork.js | 13 +- .../shared/fiber/ReactFiberCompleteWork.js | 9 +- .../shared/fiber/ReactFiberReconciler.js | 16 +- .../shared/fiber/ReactFiberUpdateQueue.js | 376 +++++++++--------- .../__tests__/ReactIncrementalUpdates-test.js | 2 +- 9 files changed, 217 insertions(+), 240 deletions(-) diff --git a/examples/fiber/index.html b/examples/fiber/index.html index 44df6da417..c965d2f781 100644 --- a/examples/fiber/index.html +++ b/examples/fiber/index.html @@ -115,14 +115,12 @@ SierpinskiTriangle.shouldComponentUpdate = function(oldProps, newProps) { var o = oldProps; var n = newProps; - const res = !( + return !( o.x === n.x && o.y === n.y && o.s === n.s && o.children === n.children ); - // console.log('shouldComponentUpdate', res); - return res; }; class ExampleApplication extends React.Component { diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index 8804b14f3e..946be9f3b4 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -286,23 +286,18 @@ var ReactNoop = { log( ' '.repeat(depth + 1) + 'QUEUED UPDATES' ); - let firstPendingUpdate; - if (updateQueue.lastProgressedUpdate) { - firstPendingUpdate = updateQueue.lastProgressedUpdate.next; - } else { - firstPendingUpdate = updateQueue.first; - } - if (!firstPendingUpdate) { + const firstUpdate = updateQueue.first; + if (!firstUpdate) { return; } log( ' '.repeat(depth + 1) + '~', - firstPendingUpdate && firstPendingUpdate.partialState, - firstPendingUpdate.callback ? 'with callback' : '' + firstUpdate && firstUpdate.partialState, + firstUpdate.callback ? 'with callback' : '' ); var next; - while (next = firstPendingUpdate.next) { + while (next = firstUpdate.next) { log( ' '.repeat(depth + 1) + '~', next.partialState, diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 5145e1a995..3b9a88fa88 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -43,6 +43,10 @@ var { NoEffect, } = require('ReactTypeOfSideEffect'); +var { + cloneUpdateQueue, +} = require('ReactFiberUpdateQueue'); + var invariant = require('invariant'); // A Fiber is work on a Component that needs to be done or was done. There can @@ -103,6 +107,8 @@ export type Fiber = { // A queue of state updates and callbacks. updateQueue: UpdateQueue | null, + // A list of callbacks that should be called during the next commit. + callbackList: UpdateQueue | null, // The state used to create the output memoizedState: any, @@ -193,6 +199,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber { pendingProps: null, memoizedProps: null, updateQueue: null, + callbackList: null, memoizedState: null, effectTag: NoEffect, @@ -268,7 +275,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi // pendingProps is here for symmetry but is unnecessary in practice for now. // TODO: Pass in the new pendingProps as an argument maybe? alt.pendingProps = fiber.pendingProps; - alt.updateQueue = fiber.updateQueue; + cloneUpdateQueue(alt, fiber); alt.pendingWorkPriority = priorityLevel; alt.memoizedProps = fiber.memoizedProps; diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 35f948231b..92179a0806 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -19,7 +19,6 @@ var { getMaskedContext, } = require('ReactFiberContext'); var { - ensureUpdateQueue, addUpdate, addReplaceUpdate, addForceUpdate, @@ -46,30 +45,26 @@ module.exports = function( isMounted, enqueueSetState(instance, partialState) { const fiber = ReactInstanceMap.get(instance); - const queue = ensureUpdateQueue(fiber); const priorityLevel = getPriorityContext(); - addUpdate(queue, partialState, priorityLevel); + addUpdate(fiber, partialState, priorityLevel); scheduleUpdateAtPriority(fiber, priorityLevel); }, enqueueReplaceState(instance, state) { const fiber = ReactInstanceMap.get(instance); - const queue = ensureUpdateQueue(fiber); const priorityLevel = getPriorityContext(); - addReplaceUpdate(queue, state, priorityLevel); + addReplaceUpdate(fiber, state, priorityLevel); scheduleUpdateAtPriority(fiber, priorityLevel); }, enqueueForceUpdate(instance) { const fiber = ReactInstanceMap.get(instance); - const queue = ensureUpdateQueue(fiber); const priorityLevel = getPriorityContext(); - addForceUpdate(queue, priorityLevel); + addForceUpdate(fiber, priorityLevel); scheduleUpdateAtPriority(fiber, priorityLevel); }, enqueueCallback(instance, callback) { const fiber = ReactInstanceMap.get(instance); - const queue = ensureUpdateQueue(fiber); const priorityLevel = getPriorityContext(); - addCallback(queue, callback, priorityLevel); + addCallback(fiber, callback, priorityLevel); scheduleUpdateAtPriority(fiber, priorityLevel); }, }; diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 9c1bf2a9f1..879fcf2485 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -25,7 +25,7 @@ var { HostPortal, CoroutineComponent, } = ReactTypeOfWork; -var { commitUpdateQueue } = require('ReactFiberUpdateQueue'); +var { commitCallbacks } = require('ReactFiberUpdateQueue'); var { Placement, @@ -417,16 +417,17 @@ module.exports = function( } attachRef(current, finishedWork, instance); } - if (finishedWork.updateQueue) { - commitUpdateQueue(finishedWork, finishedWork.updateQueue, instance); + const callbackList = finishedWork.callbackList; + if (callbackList) { + commitCallbacks(finishedWork, callbackList, instance); } return; } case HostRoot: { - const updateQueue = finishedWork.updateQueue; - if (updateQueue) { + const callbackList = finishedWork.callbackList; + if (callbackList) { const instance = finishedWork.child && finishedWork.child.stateNode; - commitUpdateQueue(finishedWork, updateQueue, instance); + commitCallbacks(finishedWork, callbackList, instance); } return; } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 93e61ce0c1..9c3a05501a 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -173,7 +173,7 @@ module.exports = function( case FunctionalComponent: workInProgress.memoizedProps = workInProgress.pendingProps; return null; - case ClassComponent: + case ClassComponent: { // We are leaving this subtree, so pop context if any. if (isContextProvider(workInProgress)) { popContextProvider(); @@ -182,11 +182,12 @@ module.exports = function( // merged it and assigned it to the instance. Transfer it from there. // Also need to transfer the props, because pendingProps will be null // in the case of an update. - const { state, props } = workInProgress.stateNode; - workInProgress.memoizedState = state; - workInProgress.memoizedProps = props; + const instance = workInProgress.stateNode; + workInProgress.memoizedState = instance.state; + workInProgress.memoizedProps = instance.props; return null; + } case HostRoot: { workInProgress.memoizedProps = workInProgress.pendingProps; const fiberRoot = (workInProgress.stateNode : FiberRoot); diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 3547470ab0..47f371b5e4 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -24,7 +24,7 @@ var { var { createFiberRoot } = require('ReactFiberRoot'); var ReactFiberScheduler = require('ReactFiberScheduler'); -var { ensureUpdateQueue, addCallback } = require('ReactFiberUpdateQueue'); +var { addCallback } = require('ReactFiberUpdateQueue'); if (__DEV__) { var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); @@ -123,13 +123,8 @@ module.exports = function(config : HostConfig(config : HostConfig | null, priorityLevel : PriorityLevel ) : void { - const update : Update = { + const update = { priorityLevel, partialState, callback: null, @@ -200,16 +228,16 @@ function addUpdate( isForced: false, next: null, }; - insertUpdateIntoQueue(queue, update); + insertUpdate(fiber, update); } exports.addUpdate = addUpdate; function addReplaceUpdate( - queue : UpdateQueue, + fiber : Fiber, state : any | null, priorityLevel : PriorityLevel ) : void { - const update : Update = { + const update = { priorityLevel, partialState: state, callback: null, @@ -217,29 +245,12 @@ function addReplaceUpdate( isForced: false, next: null, }; - // Add a pending update to the end of the queue. - if (!queue.last) { - // The queue is empty. - queue.first = queue.last = update; - return; - } - // The queue is not empty. Insert the new update into the queue, sorted by - // priority then insertion order. Since this is a replace, drop all pending - // updates with equal priority. We can't drop updates with higher priority, - // because they might be flushed in an earlier commit. We'll drop them during - // the commit phase if necessary. - const firstPendingUpdate = getFirstPendingUpdate(queue); - if (!firstPendingUpdate && queue.last) { - // This is the first pending update. Add it to the end of the queue. - queue.last.next = update; - queue.last = update; - return; - } - // Find the last pending update with equal priority. - let replaceAfter = queue.lastProgressedUpdate; - let replaceBefore = firstPendingUpdate; - if (replaceBefore) { + // Drop all updates with equal priority + let queue = ensureUpdateQueue(fiber); + for (let i = 0; queue && i < 2; i++) { + let replaceAfter = null; + let replaceBefore = queue.first; let comparison = Infinity; while (replaceBefore && (comparison = comparePriority(replaceBefore.priorityLevel, priorityLevel)) <= 0) { @@ -248,26 +259,30 @@ function addReplaceUpdate( } replaceBefore = replaceBefore.next; } + + if (replaceAfter) { + replaceAfter.next = replaceBefore; + } else { + queue.first = replaceBefore; + } + + if (!replaceBefore) { + queue.last = replaceAfter; + } + + if (fiber.alternate) { + queue = ensureUpdateQueue(fiber.alternate); + } else { + queue = null; + } } - if (replaceAfter) { - replaceAfter.next = update; - } else { - // This is the first item in the queue. - queue.first = update; - } - - if (replaceBefore) { - update.next = replaceBefore; - } else { - // This is the last item in the queue. - queue.last = update; - } + insertUpdate(fiber, update); } exports.addReplaceUpdate = addReplaceUpdate; -function addForceUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : void { - const update : Update = { +function addForceUpdate(fiber : Fiber, priorityLevel : PriorityLevel) : void { + const update = { priorityLevel, partialState: null, callback: null, @@ -275,23 +290,13 @@ function addForceUpdate(queue : UpdateQueue, priorityLevel : PriorityLevel) : vo isForced: true, next: null, }; - insertUpdateIntoQueue(queue, update); + insertUpdate(fiber, update); } exports.addForceUpdate = addForceUpdate; -function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : PriorityLevel) : void { - if (getFirstPendingUpdate(queue) && - queue.last && - (queue.last.priorityLevel === priorityLevel) && - !queue.last.callback) { - // If pending updates already exist, and the last pending update does not - // have a callback, and the priority levels are equal, we can add the - // incoming callback to that update to avoid an extra allocation. - queue.last.callback = callback; - return; - } - +function addCallback(fiber : Fiber, callback: Callback, priorityLevel : PriorityLevel) : void { + // TODO: Fast path where last item in queue has equal priority. const update : Update = { priorityLevel, partialState: null, @@ -300,13 +305,12 @@ function addCallback(queue : UpdateQueue, callback: Callback, priorityLevel : Pr isForced: false, next: null, }; - insertUpdateIntoQueue(queue, update); + insertUpdate(fiber, update); } exports.addCallback = addCallback; function getPendingPriority(queue : UpdateQueue) : PriorityLevel { - const firstPendingUpdate = getFirstPendingUpdate(queue); - return firstPendingUpdate ? firstPendingUpdate.priorityLevel : NoWork; + return queue.first ? queue.first.priorityLevel : NoWork; } exports.getPendingPriority = getPendingPriority; @@ -329,24 +333,14 @@ function beginUpdateQueue( priorityLevel : PriorityLevel ) : any { // Applies updates with matching priority to the previous state to create - // a new state object. If an update was used previously but never flushed - // due to a bail out, it's used again regardless of its priority. + // a new state object. - // Reset these flags. We'll update them while looping through the queue. - workInProgress.effectTag &= ~ForceUpdate; - workInProgress.effectTag &= ~CallbackEffect; - - const prevLastProgressedUpdate = queue.lastProgressedUpdate; let state = prevState; let dontMutatePrevState = true; let isEmpty = true; - let alreadyProgressedUpdate = Boolean(prevLastProgressedUpdate); - let lastProgressedUpdate = null; - let update : Update | null = queue.first; - while (update && ( - alreadyProgressedUpdate || - comparePriority(update.priorityLevel, priorityLevel) <= 0 - )) { + let callbackList = null; + let update = queue.first; + while (update && comparePriority(update.priorityLevel, priorityLevel) <= 0) { let partialState; if (update.isReplace) { // A replace should drop all previous updates in the queue, so @@ -370,56 +364,52 @@ function beginUpdateQueue( workInProgress.effectTag |= ForceUpdate; } if (update.callback) { + if (callbackList && callbackList.last) { + callbackList.last.next = update; + callbackList.last = update; + } else { + callbackList = { + first: update, + last: update, + }; + } workInProgress.effectTag |= CallbackEffect; } - if (update === prevLastProgressedUpdate) { - alreadyProgressedUpdate = false; - } - lastProgressedUpdate = update; update = update.next; } - - // Mark the point in the queue where we stopped applying updates - queue.lastProgressedUpdate = lastProgressedUpdate; - if (isEmpty) { - // None of the updates contained state. Return the original state object. - return prevState; + // None of the updates contained state. Use the original state object. + state = prevState; } + if (update) { + queue.first = update; + } else { + // Queue is now empty + workInProgress.updateQueue = null; + } + + workInProgress.callbackList = callbackList; + workInProgress.memoizedState = state; + return state; } exports.beginUpdateQueue = beginUpdateQueue; -function commitUpdateQueue(finishedWork : Fiber, queue : UpdateQueue, context : mixed) { - - if (finishedWork.effectTag & CallbackEffect) { - // Call the callbacks on all the non-pending updates. - let update = getFirstProgressedUpdate(queue); - while (update && update !== getFirstPendingUpdate(queue)) { - const callback = update.callback; - if (typeof callback === 'function') { - callback.call(context); - } - update = update.next; +function commitCallbacks(finishedWork : Fiber, callbackList : UpdateQueue, context : mixed) { + const stopAfter = callbackList.last; + let update = callbackList.first; + while (update) { + const callback = update.callback; + if (typeof callback === 'function') { + callback.call(context); } - } - - // Drop all comitted updates, leaving only the pending updates. - queue.first = getFirstPendingUpdate(queue); - queue.lastProgressedUpdate = null; - if (!queue.first) { - queue.last = queue.lastProgressedUpdate = null; - - // If the list is now empty, we can remove it from the finished work - finishedWork.updateQueue = null; - if (finishedWork.alternate) { - // Normally we don't mutate the current tree, but we do for updates. - // The queue on the work in progress is always the same as the queue - // on the current. - finishedWork.alternate.updateQueue = null; + if (update === stopAfter) { + break; } + update = update.next; } + finishedWork.callbackList = null; } -exports.commitUpdateQueue = commitUpdateQueue; +exports.commitCallbacks = commitCallbacks; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js index 7dbf05ad52..857a0f7e60 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js @@ -211,7 +211,7 @@ describe('ReactIncrementalUpdates', () => { // Because e is a replaceState, d gets dropped. ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span('efg')]); - // Ensure that update d is not progressed before being replaced. + // Ensure that updater function d is never called. expect(progressedUpdates).toEqual(['e', 'f', 'g']); }); From babace0c059965b01767797adace4e765a4650d7 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 13 Dec 2016 10:55:55 -0800 Subject: [PATCH 09/16] Add fast path for appending updates to the end of the queue This is the most common case, so we should avoid scanning the entire list to get to the end. --- .../shared/fiber/ReactFiberUpdateQueue.js | 57 +++++++++++-------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 9938db1eed..9c9aadccbd 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -176,26 +176,41 @@ function insertUpdate(fiber : Fiber, update : Update) : void { const priorityLevel = update.priorityLevel; - // TODO: Fast path where last item in the queue has greater or equal priority. - - let insertAfter1 = null; - let insertBefore1 = queue1.first; - while (insertBefore1 && comparePriority(insertBefore1.priorityLevel, priorityLevel) <= 0) { - insertAfter1 = insertBefore1; - insertBefore1 = insertBefore1.next; - } - const update1 = update; - - let insertAfter2 = null; - let insertBefore2 = null; - let update2 = null; - if (queue2) { - insertBefore2 = queue2.first; - while (insertBefore2 && comparePriority(insertBefore2.priorityLevel, priorityLevel) <= 0) { - insertAfter2 = insertBefore2; - insertBefore2 = insertBefore2.next; + let queue = queue1; + let insertAfter1; + let insertBefore1; + let insertAfter2; + let insertBefore2; + for (let i = 0; queue && i < 2; i++) { + let insertAfter = null; + let insertBefore = null; + if (queue.last && comparePriority(queue.last.priorityLevel, priorityLevel) <= 0) { + // Fast path for the common case where the update should be inserted at + // the end of the queue. + insertAfter = queue.last; + } else { + insertBefore = queue.first; + while (insertBefore && comparePriority(insertBefore.priorityLevel, priorityLevel) <= 0) { + insertAfter = insertBefore; + insertBefore = insertBefore.next; + } } + if (i === 0) { + insertAfter1 = insertAfter; + insertBefore1 = insertBefore; + queue = queue2; + } else { + insertAfter2 = insertAfter; + insertBefore2 = insertBefore; + queue = null; + } + } + const update1 = update; + insertUpdateIntoQueue(queue1, update1, insertAfter1, insertBefore1); + + if (queue2) { + let update2; if (insertBefore1 === insertBefore2) { // The update is inserted into the same position of both lists. There's no // need to clone the update. @@ -206,11 +221,6 @@ function insertUpdate(fiber : Fiber, update : Update) : void { // time we commit. update2 = cloneUpdate(update1); } - } - - // Now we're ready to insert the updates into their queues. - insertUpdateIntoQueue(queue1, update1, insertAfter1, insertBefore1); - if (queue2 && update2) { insertUpdateIntoQueue(queue2, update2, insertAfter2, insertBefore2); } } @@ -296,7 +306,6 @@ exports.addForceUpdate = addForceUpdate; function addCallback(fiber : Fiber, callback: Callback, priorityLevel : PriorityLevel) : void { - // TODO: Fast path where last item in queue has equal priority. const update : Update = { priorityLevel, partialState: null, From e0981b8bc5c30ecbb56bb116161993b79667e607 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 13 Dec 2016 13:57:54 -0800 Subject: [PATCH 10/16] Handle setState inside an updater function The update is scheduled as if the current processing update has already been processed; if it has the same or higher priority, it will be flushed in the same batch. We also print a warning. --- scripts/fiber/tests-passing.txt | 1 + .../shared/fiber/ReactFiberUpdateQueue.js | 86 ++++++++++++++++--- .../__tests__/ReactIncrementalUpdates-test.js | 63 ++++++++++++++ 3 files changed, 137 insertions(+), 13 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index ebe26c9d9d..91f4eecbc1 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1237,6 +1237,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js * can abort an update, schedule additional updates, and resume * can abort an update, schedule a replaceState, and resume * does not call callbacks that are scheduled by another callback until a later commit +* enqueues setState inside an updater function as if the in-progress update is progressed (and warns) src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js * should render a simple fragment at the top of a component diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 9c9aadccbd..5df5520b15 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -55,6 +55,9 @@ type Update = { export type UpdateQueue = { first: Update | null, last: Update | null, + + // Dev only + isProcessing?: boolean, }; function comparePriority(a : PriorityLevel, b : PriorityLevel) : number { @@ -90,10 +93,21 @@ function ensureUpdateQueue(fiber : Fiber) : UpdateQueue { // We already have an update queue. return fiber.updateQueue; } - const queue = { - first: null, - last: null, - }; + + let queue; + if (__DEV__) { + queue = { + first: null, + last: null, + isProcessing: false, + }; + } else { + queue = { + first: null, + last: null, + }; + } + fiber.updateQueue = queue; return queue; } @@ -170,10 +184,30 @@ function insertUpdateIntoQueue(queue, update, insertAfter, insertBefore) { // // However, if incoming update is inserted into the same position of both lists, // we shouldn't make a copy. -function insertUpdate(fiber : Fiber, update : Update) : void { + +function insertUpdate(fiber : Fiber, update : Update, methodName : ?string) : void { const queue1 = ensureUpdateQueue(fiber); const queue2 = fiber.alternate ? ensureUpdateQueue(fiber.alternate) : null; + // Warn if an update is scheduled from inside an updater function. + if (__DEV__ && typeof methodName === 'string' && (queue1.isProcessing || (queue2 && queue2.isProcessing))) { + if (methodName === 'setState') { + console.error( + 'setState was called from inside the updater function of another' + + 'setState. A function passed as the first argument of setState ' + + 'should not contain any side-effects. Return a partial state object ' + + 'instead of calling setState again. Example: ' + + 'this.setState(function(state) { return { count: state.count + 1 }; })' + ); + } else { + console.error( + `${methodName} was called from inside the updater function of ` + + 'setState. A function passed as the first argument of setState ' + + 'should not contain any side-effects.' + ); + } + } + const priorityLevel = update.priorityLevel; let queue = queue1; @@ -238,7 +272,11 @@ function addUpdate( isForced: false, next: null, }; - insertUpdate(fiber, update); + if (__DEV__) { + insertUpdate(fiber, update, 'setState'); + } else { + insertUpdate(fiber, update); + } } exports.addUpdate = addUpdate; @@ -286,8 +324,11 @@ function addReplaceUpdate( queue = null; } } - - insertUpdate(fiber, update); + if (__DEV__) { + insertUpdate(fiber, update, 'replaceState'); + } else { + insertUpdate(fiber, update); + } } exports.addReplaceUpdate = addReplaceUpdate; @@ -300,7 +341,11 @@ function addForceUpdate(fiber : Fiber, priorityLevel : PriorityLevel) : void { isForced: true, next: null, }; - insertUpdate(fiber, update); + if (__DEV__) { + insertUpdate(fiber, update, 'forceUpdate'); + } else { + insertUpdate(fiber, update); + } } exports.addForceUpdate = addForceUpdate; @@ -341,15 +386,28 @@ function beginUpdateQueue( props : any, priorityLevel : PriorityLevel ) : any { + if (__DEV__) { + // Set this flag so we can warn if setState is called inside the update + // function of another setState. + queue.isProcessing = true; + } + // Applies updates with matching priority to the previous state to create // a new state object. - let state = prevState; let dontMutatePrevState = true; let isEmpty = true; let callbackList = null; let update = queue.first; while (update && comparePriority(update.priorityLevel, priorityLevel) <= 0) { + // Remove each update from the queue right before it is processed. That way + // if setState is called from inside an updater function, the new update + // will be inserted in the correct position. + queue.first = update.next; + if (!queue.first) { + queue.last = null; + } + let partialState; if (update.isReplace) { // A replace should drop all previous updates in the queue, so @@ -392,9 +450,7 @@ function beginUpdateQueue( state = prevState; } - if (update) { - queue.first = update; - } else { + if (!queue.first) { // Queue is now empty workInProgress.updateQueue = null; } @@ -402,6 +458,10 @@ function beginUpdateQueue( workInProgress.callbackList = callbackList; workInProgress.memoizedState = state; + if (__DEV__) { + queue.isProcessing = false; + } + return state; } exports.beginUpdateQueue = beginUpdateQueue; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js index 857a0f7e60..af1f9621c5 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js @@ -245,4 +245,67 @@ describe('ReactIncrementalUpdates', () => { 'callback b', ]); }); + + it('enqueues setState inside an updater function as if the in-progress update is progressed (and warns)', () => { + spyOn(console, 'error'); + let instance; + let ops = []; + class Foo extends React.Component { + state = {}; + componentDidMount() { + ops.push('componentDidMount'); + this.setState(function a() { + // Force update b to have Task priority + ReactNoop.syncUpdates(() => { + this.setState({ b: 'b' }); + }); + return { a: 'a' }; + }); + } + render() { + ops.push('render'); + instance = this; + return ; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expectDev(console.error.calls.count()).toBe(1); + expect(ReactNoop.getChildren()).toEqual([span('ab')]); + expect(ops).toEqual([ + // Initial render + 'render', + 'componentDidMount', + // Updates a and b both have Task priority. Update b is enqueued while + // update a is being processed, but it should be inserted into the queue + // as if update a is already processed. Then processing continues. Because + // they have the same priority, update b is processed in the same batch. + // So there should only be a single render below. + 'render', + ]); + + ops = []; + + ReactNoop.performAnimationWork(() => { + instance.setState(function c() { + // Update d happens during the begin phase, so it has low priority. + this.setState({ d: 'd' }); + return { c: 'c' }; + }); + }); + + ReactNoop.flush(); + expect(ReactNoop.getChildren()).toEqual([span('abcd')]); + expect(ops).toEqual([ + // Update c has animation priority. Update d is enqueued while c is being + // processed with animation priority. Because d is low priority, it is not + // processed until the next render. So there should be two renders below. + 'render', + 'render', + ]); + + expectDev(console.error.calls.count()).toBe(2); + console.error.calls.reset(); + }); }); From eec431297a145ccc0159ce3a1bf093e47c9c88d5 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 13 Dec 2016 15:05:56 -0800 Subject: [PATCH 11/16] Priority context during reconciliation setState inside render/cWRP should have the same priority as whatever level is currently being reconciled. --- scripts/fiber/tests-passing.txt | 1 + .../shared/fiber/ReactFiberBeginWork.js | 13 ++- .../shared/fiber/ReactFiberClassComponent.js | 26 ++---- .../shared/fiber/ReactFiberReconciler.js | 10 +-- .../shared/fiber/ReactFiberScheduler.js | 68 ++++++++++++--- .../__tests__/ReactIncrementalUpdates-test.js | 85 ++++++++++--------- 6 files changed, 125 insertions(+), 78 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 91f4eecbc1..5b821222ca 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1237,6 +1237,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js * can abort an update, schedule additional updates, and resume * can abort an update, schedule a replaceState, and resume * does not call callbacks that are scheduled by another callback until a later commit +* gives setState during reconciliation the same priority as whatever level is currently reconciling * enqueues setState inside an updater function as if the in-progress update is progressed (and warns) src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index f00bcca831..94799919b9 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -71,8 +71,10 @@ if (__DEV__) { module.exports = function( config : HostConfig, hostContext : HostContext, - scheduleUpdateAtPriority : (fiber: Fiber, priorityLevel : PriorityLevel) => void, - getPriorityContext : () => PriorityLevel + scheduleSetState: (fiber : Fiber, partialState : any) => void, + scheduleReplaceState: (fiber : Fiber, state : any) => void, + scheduleForceUpdate: (fiber : Fiber) => void, + scheduleUpdateCallback: (fiber : Fiber, callback : Function) => void, ) { const { shouldSetTextContent } = config; @@ -89,7 +91,12 @@ module.exports = function( mountClassInstance, resumeMountClassInstance, updateClassInstance, - } = ReactFiberClassComponent(scheduleUpdateAtPriority, getPriorityContext); + } = ReactFiberClassComponent( + scheduleSetState, + scheduleReplaceState, + scheduleForceUpdate, + scheduleUpdateCallback + ); function markChildAsProgressed(current, workInProgress, priorityLevel) { // We now have clones. Let's store them as the currently progressed work. diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 92179a0806..1852b52aba 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -19,10 +19,6 @@ var { getMaskedContext, } = require('ReactFiberContext'); var { - addUpdate, - addReplaceUpdate, - addForceUpdate, - addCallback, beginUpdateQueue, } = require('ReactFiberUpdateQueue'); var { hasContextChanged } = require('ReactFiberContext'); @@ -36,8 +32,10 @@ var invariant = require('invariant'); const isArray = Array.isArray; module.exports = function( - scheduleUpdateAtPriority : (fiber: Fiber, priorityLevel : PriorityLevel) => void, - getPriorityContext : () => PriorityLevel, + scheduleSetState: (fiber : Fiber, partialState : any) => void, + scheduleReplaceState: (fiber : Fiber, state : any) => void, + scheduleForceUpdate: (fiber : Fiber) => void, + scheduleUpdateCallback: (fiber : Fiber, callback : Function) => void, ) { // Class component state updater @@ -45,27 +43,19 @@ module.exports = function( isMounted, enqueueSetState(instance, partialState) { const fiber = ReactInstanceMap.get(instance); - const priorityLevel = getPriorityContext(); - addUpdate(fiber, partialState, priorityLevel); - scheduleUpdateAtPriority(fiber, priorityLevel); + scheduleSetState(fiber, partialState); }, enqueueReplaceState(instance, state) { const fiber = ReactInstanceMap.get(instance); - const priorityLevel = getPriorityContext(); - addReplaceUpdate(fiber, state, priorityLevel); - scheduleUpdateAtPriority(fiber, priorityLevel); + scheduleReplaceState(fiber, state); }, enqueueForceUpdate(instance) { const fiber = ReactInstanceMap.get(instance); - const priorityLevel = getPriorityContext(); - addForceUpdate(fiber, priorityLevel); - scheduleUpdateAtPriority(fiber, priorityLevel); + scheduleForceUpdate(fiber); }, enqueueCallback(instance, callback) { const fiber = ReactInstanceMap.get(instance); - const priorityLevel = getPriorityContext(); - addCallback(fiber, callback, priorityLevel); - scheduleUpdateAtPriority(fiber, priorityLevel); + scheduleUpdateCallback(fiber, callback); }, }; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 47f371b5e4..177fc674dc 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -24,8 +24,6 @@ var { var { createFiberRoot } = require('ReactFiberRoot'); var ReactFiberScheduler = require('ReactFiberScheduler'); -var { addCallback } = require('ReactFiberUpdateQueue'); - if (__DEV__) { var ReactFiberInstrumentation = require('ReactFiberInstrumentation'); } @@ -100,7 +98,7 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig) { const hostContext = ReactFiberHostContext(config); const { popHostContainer, popHostContext, resetHostContainer } = hostContext; - const { beginWork, beginFailedWork } = - ReactFiberBeginWork(config, hostContext, scheduleUpdateAtPriority, getPriorityContext); + const { beginWork, beginFailedWork } = ReactFiberBeginWork( + config, + hostContext, + scheduleSetState, + scheduleReplaceState, + scheduleForceUpdate, + scheduleUpdateCallback, + ); const { completeWork } = ReactFiberCompleteWork(config, hostContext); const { commitPlacement, @@ -95,6 +105,10 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig { ]); }); + it('gives setState during reconciliation the same priority as whatever level is currently reconciling', () => { + let instance; + let ops = []; + + class Foo extends React.Component { + state = {}; + componentWillReceiveProps() { + ops.push('componentWillReceiveProps'); + this.setState({ b: 'b' }); + } + render() { + ops.push('render'); + instance = this; + return ; + } + } + ReactNoop.render(); + ReactNoop.flush(); + + ops = []; + + ReactNoop.performAnimationWork(() => { + instance.setState({ a: 'a' }); + ReactNoop.render(); // Trigger componentWillReceiveProps + }); + ReactNoop.flush(); + + expect(ReactNoop.getChildren()).toEqual([span('ab')]); + expect(ops).toEqual([ + 'componentWillReceiveProps', + 'render', + ]); + }); + + it('enqueues setState inside an updater function as if the in-progress update is progressed (and warns)', () => { spyOn(console, 'error'); let instance; let ops = []; class Foo extends React.Component { state = {}; - componentDidMount() { - ops.push('componentDidMount'); - this.setState(function a() { - // Force update b to have Task priority - ReactNoop.syncUpdates(() => { - this.setState({ b: 'b' }); - }); - return { a: 'a' }; - }); - } render() { ops.push('render'); instance = this; @@ -271,41 +296,25 @@ describe('ReactIncrementalUpdates', () => { ReactNoop.render(); ReactNoop.flush(); - expectDev(console.error.calls.count()).toBe(1); + + instance.setState(function a() { + ops.push('setState updater'); + this.setState({ b: 'b' }); + return { a: 'a' }; + }); + + ReactNoop.flush(); expect(ReactNoop.getChildren()).toEqual([span('ab')]); expect(ops).toEqual([ // Initial render 'render', - 'componentDidMount', - // Updates a and b both have Task priority. Update b is enqueued while - // update a is being processed, but it should be inserted into the queue - // as if update a is already processed. Then processing continues. Because - // they have the same priority, update b is processed in the same batch. - // So there should only be a single render below. + 'setState updater', + // Update b is enqueued with the same priority as update a, so it should + // be flushed in the same commit. 'render', ]); - ops = []; - - ReactNoop.performAnimationWork(() => { - instance.setState(function c() { - // Update d happens during the begin phase, so it has low priority. - this.setState({ d: 'd' }); - return { c: 'c' }; - }); - }); - - ReactNoop.flush(); - expect(ReactNoop.getChildren()).toEqual([span('abcd')]); - expect(ops).toEqual([ - // Update c has animation priority. Update d is enqueued while c is being - // processed with animation priority. Because d is low priority, it is not - // processed until the next render. So there should be two renders below. - 'render', - 'render', - ]); - - expectDev(console.error.calls.count()).toBe(2); + expectDev(console.error.calls.count()).toBe(1); console.error.calls.reset(); }); }); From df9603ead8c49f3638503f92ff56fcf07d137079 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 13 Dec 2016 15:49:26 -0800 Subject: [PATCH 12/16] Simplify test for whether we should flush a pending commit We can just check if the deadline has expired. --- src/renderers/shared/fiber/ReactFiberScheduler.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 316edeaa18..9eff3bfcf9 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -680,17 +680,9 @@ module.exports = function(config : HostConfig Date: Tue, 13 Dec 2016 19:07:30 -0800 Subject: [PATCH 13/16] Move priority context change to findNextUnitOfWork ...rather than changing it on every unit of work. --- src/renderers/shared/fiber/ReactFiberScheduler.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 9eff3bfcf9..21feb4bc2f 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -159,6 +159,9 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig(config : HostConfig Date: Tue, 13 Dec 2016 20:09:19 -0800 Subject: [PATCH 14/16] Update tests to not rely on key iteration order --- .../shared/fiber/ReactFiberScheduler.js | 3 +- .../__tests__/ReactIncrementalUpdates-test.js | 106 ++++++++++++------ 2 files changed, 72 insertions(+), 37 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 21feb4bc2f..11078acf06 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -390,7 +390,8 @@ module.exports = function(config : HostConfig { ReactNoop = require('ReactNoop'); }); - function div(...children) { - children = children.map(c => typeof c === 'string' ? { text: c } : c); - return { type: 'div', children, prop: undefined }; - } - - function span(prop) { - return { type: 'span', children: [], prop }; - } - it('applies updates in order of priority', () => { let state; class Foo extends React.Component { @@ -50,8 +41,10 @@ describe('ReactIncrementalUpdates', () => { } ReactNoop.render(); + ReactNoop.flushDeferredPri(25); + expect(state).toEqual({ a: 'a' }); ReactNoop.flush(); - expect(Object.keys(state)).toEqual(['a', 'b', 'c']); + expect(state).toEqual({ a: 'a', b: 'b', c: 'c' }); }); it('applies updates with equal priority in insertion order', () => { @@ -72,20 +65,26 @@ describe('ReactIncrementalUpdates', () => { ReactNoop.render(); ReactNoop.flush(); - expect(Object.keys(state)).toEqual(['a', 'b', 'c']); + expect(state).toEqual({ a: 'a', b: 'b', c: 'c' }); }); it('only drops updates with equal or lesser priority when replaceState is called', () => { let instance; + let ops = []; const Foo = React.createClass({ getInitialState() { return {}; }, + componentDidMount() { + ops.push('componentDidMount'); + }, + componentDidUpdate() { + ops.push('componentDidUpdate'); + }, render() { + ops.push('render'); instance = this; - return ( - - ); + return
; }, }); @@ -104,28 +103,45 @@ describe('ReactIncrementalUpdates', () => { ReactNoop.flushAnimationPri(); // Even though a replaceState has been already scheduled, it hasn't been // flushed yet because it has low priority. - expect(ReactNoop.getChildren()).toEqual([span('ab')]); + expect(instance.state).toEqual({ a: 'a', b: 'b' }); + expect(ops).toEqual([ + 'render', + 'componentDidMount', + 'render', + 'componentDidUpdate', + ]); + + ops = []; ReactNoop.flush(); // Now the rest of the updates are flushed. - expect(ReactNoop.getChildren()).toEqual([span('cd')]); + expect(instance.state).toEqual({ c: 'c', d: 'd' }); + expect(ops).toEqual([ + 'render', + 'componentDidUpdate', + ]); }); it('can abort an update, schedule additional updates, and resume', () => { let instance; + let ops = []; class Foo extends React.Component { state = {}; + componentDidUpdate() { + ops.push('componentDidUpdate'); + } render() { + ops.push('render'); instance = this; - return ( - - ); + return
; } } ReactNoop.render(); ReactNoop.flush(); + ops = []; + let progressedUpdates = []; function createUpdate(letter) { return () => { @@ -141,9 +157,14 @@ describe('ReactIncrementalUpdates', () => { instance.setState(createUpdate('c')); // Do just enough work to begin the update but not enough to flush it - ReactNoop.flushDeferredPri(20); - expect(ReactNoop.getChildren()).toEqual([span('')]); + ReactNoop.flushDeferredPri(15); + // expect(ReactNoop.getChildren()).toEqual([span('')]); + expect(ops).toEqual(['render']); expect(progressedUpdates).toEqual(['a', 'b', 'c']); + expect(instance.state).toEqual({ a: 'a', b: 'b', c: 'c' }); + + ops = []; + progressedUpdates = []; instance.setState(createUpdate('f')); ReactNoop.performAnimationWork(() => { @@ -152,21 +173,35 @@ describe('ReactIncrementalUpdates', () => { }); instance.setState(createUpdate('g')); - // Updates a, b, and c were aborted, so they should be applied first even - // though they have low priority. Update f was scheduled after the render - // was aborted, so it should come after d and e, which have higher priority. + ReactNoop.flushAnimationPri(); + expect(ops).toEqual([ + // Flushes animation work (d and e) + 'render', + 'componentDidUpdate', + ]); + ops = []; ReactNoop.flush(); - expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]); - expect(progressedUpdates).toEqual(['a', 'b', 'c', 'd', 'e', 'f', 'g']); + expect(ops).toEqual([ + // Flushes deferred work (f and g) + 'render', + 'componentDidUpdate', + ]); + expect(progressedUpdates).toEqual(['d', 'e', 'a', 'b', 'c', 'f', 'g']); + expect(instance.state).toEqual({ a: 'a', b: 'b', c: 'c', d: 'd', e: 'e', f: 'f', g: 'g' }); }); it('can abort an update, schedule a replaceState, and resume', () => { let instance; + let ops = []; const Foo = React.createClass({ getInitialState() { return {}; }, + componentDidUpdate() { + ops.push('componentDidUpdate'); + }, render() { + ops.push('render'); instance = this; return ( @@ -176,6 +211,7 @@ describe('ReactIncrementalUpdates', () => { ReactNoop.render(); ReactNoop.flush(); + ops = []; let progressedUpdates = []; function createUpdate(letter) { @@ -193,9 +229,11 @@ describe('ReactIncrementalUpdates', () => { // Do just enough work to begin the update but not enough to flush it ReactNoop.flushDeferredPri(20); - expect(ReactNoop.getChildren()).toEqual([span('')]); + expect(ops).toEqual(['render']); expect(progressedUpdates).toEqual(['a', 'b', 'c']); + expect(instance.state).toEqual({ a: 'a', b: 'b', c: 'c' }); + ops = []; progressedUpdates = []; instance.setState(createUpdate('f')); @@ -205,14 +243,10 @@ describe('ReactIncrementalUpdates', () => { }); instance.setState(createUpdate('g')); - // Updates a, b, and c were aborted, so they should be applied first even - // though they have low priority. Update f was scheduled after the render - // was aborted, so it should come after d and e, which have higher priority. - // Because e is a replaceState, d gets dropped. ReactNoop.flush(); - expect(ReactNoop.getChildren()).toEqual([span('efg')]); // Ensure that updater function d is never called. expect(progressedUpdates).toEqual(['e', 'f', 'g']); + expect(instance.state).toEqual({ e: 'e', f: 'f', g: 'g' }); }); it('does not call callbacks that are scheduled by another callback until a later commit', () => { @@ -259,7 +293,7 @@ describe('ReactIncrementalUpdates', () => { render() { ops.push('render'); instance = this; - return ; + return
; } } ReactNoop.render(); @@ -273,7 +307,7 @@ describe('ReactIncrementalUpdates', () => { }); ReactNoop.flush(); - expect(ReactNoop.getChildren()).toEqual([span('ab')]); + expect(instance.state).toEqual({ a: 'a', b: 'b' }); expect(ops).toEqual([ 'componentWillReceiveProps', 'render', @@ -290,7 +324,7 @@ describe('ReactIncrementalUpdates', () => { render() { ops.push('render'); instance = this; - return ; + return
; } } @@ -304,7 +338,6 @@ describe('ReactIncrementalUpdates', () => { }); ReactNoop.flush(); - expect(ReactNoop.getChildren()).toEqual([span('ab')]); expect(ops).toEqual([ // Initial render 'render', @@ -313,6 +346,7 @@ describe('ReactIncrementalUpdates', () => { // be flushed in the same commit. 'render', ]); + expect(instance.state).toEqual({ a: 'a', b: 'b' }); expectDev(console.error.calls.count()).toBe(1); console.error.calls.reset(); From 3a946fe0944aab319c7b73fb196314c80c955027 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 14 Dec 2016 13:42:03 -0800 Subject: [PATCH 15/16] Use 255 instead of infinity Infinity is a floating point value. --- src/renderers/shared/fiber/ReactFiberUpdateQueue.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 5df5520b15..900e3ece16 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -69,10 +69,10 @@ function comparePriority(a : PriorityLevel, b : PriorityLevel) : number { return 0; } if (a === NoWork && b !== NoWork) { - return -Infinity; + return -255; } if (a !== NoWork && b === NoWork) { - return Infinity; + return 255; } return a - b; } @@ -299,7 +299,7 @@ function addReplaceUpdate( for (let i = 0; queue && i < 2; i++) { let replaceAfter = null; let replaceBefore = queue.first; - let comparison = Infinity; + let comparison = 255; while (replaceBefore && (comparison = comparePriority(replaceBefore.priorityLevel, priorityLevel)) <= 0) { if (comparison < 0) { From b8441ba955e08f5ad0595625e233d28c04a77867 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 15 Dec 2016 16:35:54 -0800 Subject: [PATCH 16/16] Remove ForceUpdate effect Go back to using a flag, instead. I removed it before because I thought we might want to get rid of the top-level UpdateQueue type and put the fields directly on the fiber, but since we're keeping UpdateQueue we can put hasForceUpdate on there. --- .../shared/fiber/ReactFiberClassComponent.js | 5 ++--- .../shared/fiber/ReactFiberScheduler.js | 3 +-- .../shared/fiber/ReactFiberUpdateQueue.js | 12 +++++++++--- .../shared/fiber/ReactTypeOfSideEffect.js | 19 +++++++++---------- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 1852b52aba..3772571f8d 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -22,7 +22,6 @@ var { beginUpdateQueue, } = require('ReactFiberUpdateQueue'); var { hasContextChanged } = require('ReactFiberContext'); -var { ForceUpdate } = require('ReactTypeOfSideEffect'); var { getComponentName, isMounted } = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); var shallowEqual = require('shallowEqual'); @@ -60,7 +59,7 @@ module.exports = function( }; function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState, newContext) { - if (oldProps === null || (workInProgress.effectTag & ForceUpdate)) { + if (oldProps === null || (workInProgress.updateQueue && workInProgress.updateQueue.hasForceUpdate)) { // If the workInProgress already has an Update effect, return true return true; } @@ -344,7 +343,7 @@ module.exports = function( if (oldProps === newProps && oldState === newState && !hasContextChanged() && - !(workInProgress.effectTag & ForceUpdate)) { + !(updateQueue && updateQueue.hasForceUpdate)) { return false; } diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 11078acf06..88887082d6 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -40,7 +40,6 @@ var { Placement, Update, PlacementAndUpdate, - ForceUpdate, Deletion, ContentReset, Callback, @@ -224,7 +223,7 @@ module.exports = function(config : HostConfig