From 643c393de9d947f4f1bcaa032c574fffb007d735 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 11 Oct 2017 17:54:14 -0700 Subject: [PATCH 01/10] Fix ceiling function Mixed up the operators. --- src/renderers/shared/fiber/ReactFiberExpirationTime.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/shared/fiber/ReactFiberExpirationTime.js b/src/renderers/shared/fiber/ReactFiberExpirationTime.js index 202be36e37f15..49c692ea241d4 100644 --- a/src/renderers/shared/fiber/ReactFiberExpirationTime.js +++ b/src/renderers/shared/fiber/ReactFiberExpirationTime.js @@ -34,7 +34,7 @@ function msToExpirationTime(ms: number): ExpirationTime { exports.msToExpirationTime = msToExpirationTime; function ceiling(num: number, precision: number): number { - return (((((num * precision) | 0) + 1) / precision) | 0) + 1; + return (((num / precision) | 0) + 1) * precision; } function bucket( From f662d9839b5931c4f263b564e60a2845b372aa73 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 11 Oct 2017 18:40:55 -0700 Subject: [PATCH 02/10] Remove addUpdate, addReplaceState, et al These functions don't really do anything. Simpler to use a single insertUpdateIntoFiber function. Also splits scheduleUpdate into two functions: - scheduleWork traverses a fiber's ancestor path and updates their expiration times. - scheduleUpdate inserts an update into a fiber's update queue, then calls scheduleWork. --- .../shared/fiber/ReactFiberBeginWork.js | 16 +- .../shared/fiber/ReactFiberClassComponent.js | 28 ++-- .../shared/fiber/ReactFiberCommitWork.js | 34 +++- .../shared/fiber/ReactFiberReconciler.js | 39 ++++- .../shared/fiber/ReactFiberScheduler.js | 38 ++++- .../shared/fiber/ReactFiberUpdateQueue.js | 146 ++---------------- 6 files changed, 127 insertions(+), 174 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 980c3b07663d3..272f72153eb81 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -71,8 +71,13 @@ module.exports = function( config: HostConfig, hostContext: HostContext, hydrationContext: HydrationContext, - scheduleUpdate: (fiber: Fiber, expirationTime: ExpirationTime) => void, - getExpirationTime: (fiber: Fiber, forceAsync: boolean) => ExpirationTime, + scheduleUpdate: ( + fiber: Fiber, + partialState: mixed, + callback: (() => mixed) | null, + isReplace: boolean, + isForced: boolean, + ) => void, ) { const { shouldSetTextContent, @@ -94,12 +99,7 @@ module.exports = function( mountClassInstance, // resumeMountClassInstance, updateClassInstance, - } = ReactFiberClassComponent( - scheduleUpdate, - getExpirationTime, - memoizeProps, - memoizeState, - ); + } = ReactFiberClassComponent(scheduleUpdate, memoizeProps, memoizeState); // TODO: Remove this and use reconcileChildrenAtExpirationTime directly. function reconcileChildren(current, workInProgress, nextChildren) { diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 0e2182e0fc2b8..6b7cf7f8812d5 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -24,12 +24,7 @@ var { getUnmaskedContext, isContextConsumer, } = require('ReactFiberContext'); -var { - addUpdate, - addReplaceUpdate, - addForceUpdate, - beginUpdateQueue, -} = require('ReactFiberUpdateQueue'); +var {beginUpdateQueue} = require('ReactFiberUpdateQueue'); var {hasContextChanged} = require('ReactFiberContext'); var {isMounted} = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); @@ -77,8 +72,13 @@ if (__DEV__) { } module.exports = function( - scheduleUpdate: (fiber: Fiber, expirationTime: ExpirationTime) => void, - getExpirationTime: (fiber: Fiber, forceAsync: boolean) => ExpirationTime, + scheduleUpdate: ( + fiber: Fiber, + partialState: mixed, + callback: (() => mixed) | null, + isReplace: boolean, + isForced: boolean, + ) => void, memoizeProps: (workInProgress: Fiber, props: any) => void, memoizeState: (workInProgress: Fiber, state: any) => void, ) { @@ -87,33 +87,27 @@ module.exports = function( isMounted, enqueueSetState(instance, partialState, callback) { const fiber = ReactInstanceMap.get(instance); - const expirationTime = getExpirationTime(fiber, false); callback = callback === undefined ? null : callback; if (__DEV__) { warnOnInvalidCallback(callback, 'setState'); } - addUpdate(fiber, partialState, callback, expirationTime); - scheduleUpdate(fiber, expirationTime); + scheduleUpdate(fiber, partialState, callback, false, false); }, enqueueReplaceState(instance, state, callback) { const fiber = ReactInstanceMap.get(instance); - const expirationTime = getExpirationTime(fiber, false); callback = callback === undefined ? null : callback; if (__DEV__) { warnOnInvalidCallback(callback, 'replaceState'); } - addReplaceUpdate(fiber, state, callback, expirationTime); - scheduleUpdate(fiber, expirationTime); + scheduleUpdate(fiber, state, callback, true, false); }, enqueueForceUpdate(instance, callback) { const fiber = ReactInstanceMap.get(instance); - const expirationTime = getExpirationTime(fiber, false); callback = callback === undefined ? null : callback; if (__DEV__) { warnOnInvalidCallback(callback, 'forceUpdate'); } - addForceUpdate(fiber, callback, expirationTime); - scheduleUpdate(fiber, expirationTime); + scheduleUpdate(fiber, null, callback, false, true); }, }; diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 32e7fd9bc7aa3..e67f17bac2f5c 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -22,7 +22,6 @@ var { HostPortal, CoroutineComponent, } = ReactTypeOfWork; -var {commitCallbacks} = require('ReactFiberUpdateQueue'); var {onCommitUnmount} = require('ReactFiberDevToolsHook'); var { invokeGuardedCallback, @@ -103,6 +102,19 @@ module.exports = function( } } + function commitCallbacks(callbackList, context) { + for (let i = 0; i < callbackList.length; i++) { + const callback = callbackList[i]; + invariant( + typeof callback === 'function', + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: %s', + callback, + ); + callback.call(context); + } + } + function commitLifeCycles(current: Fiber | null, finishedWork: Fiber): void { switch (finishedWork.tag) { case ClassComponent: { @@ -136,15 +148,27 @@ module.exports = function( finishedWork.effectTag & Callback && finishedWork.updateQueue !== null ) { - commitCallbacks(finishedWork, finishedWork.updateQueue, instance); + const updateQueue = finishedWork.updateQueue; + if (updateQueue.callbackList !== null) { + // Set the list to null to make sure they don't get called more than once. + const callbackList = updateQueue.callbackList; + updateQueue.callbackList = null; + commitCallbacks(callbackList, instance); + } } return; } case HostRoot: { const updateQueue = finishedWork.updateQueue; - if (updateQueue !== null) { - const instance = finishedWork.child && finishedWork.child.stateNode; - commitCallbacks(finishedWork, updateQueue, instance); + if (updateQueue !== null && updateQueue.callbackList !== null) { + // Set the list to null to make sure they don't get called more + // than once. + const callbackList = updateQueue.callbackList; + updateQueue.callbackList = null; + const instance = finishedWork.child !== null + ? finishedWork.child.stateNode + : null; + commitCallbacks(callbackList, instance); } return; } diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 697f41ca3505d..a9d84bd9ddc75 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -16,7 +16,7 @@ import type {ReactNodeList} from 'ReactTypes'; var ReactFeatureFlags = require('ReactFeatureFlags'); -var {addTopLevelUpdate} = require('ReactFiberUpdateQueue'); +var {insertUpdateIntoFiber} = require('ReactFiberUpdateQueue'); var { findCurrentUnmaskedContext, @@ -265,7 +265,7 @@ module.exports = function( var {getPublicInstance} = config; var { - scheduleUpdate, + scheduleWork, getExpirationTime, batchedUpdates, unbatchedUpdates, @@ -316,8 +316,39 @@ module.exports = function( callback, ); } - addTopLevelUpdate(current, nextState, callback, expirationTime); - scheduleUpdate(current, expirationTime); + const isTopLevelUnmount = nextState.element === null; + const update = { + expirationTime, + partialState: nextState, + callback, + isReplace: false, + isForced: false, + isTopLevelUnmount, + next: null, + }; + const update2 = insertUpdateIntoFiber(current, update); + + if (isTopLevelUnmount) { + // TODO: Redesign the top-level mount/update/unmount API to avoid this + // special case. + const queue1 = current.updateQueue; + const queue2 = current.alternate !== null + ? current.alternate.updateQueue + : null; + + // Drop all updates that are lower-priority, so that the tree is not + // remounted. We need to do this for both queues. + if (queue1 !== null && update.next !== null) { + update.next = null; + queue1.last = update; + } + if (queue2 !== null && update2 !== null && update2.next !== null) { + update2.next = null; + queue2.last = update; + } + } + + scheduleWork(current, expirationTime); } return { diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 13bcea3b5b432..8c81c9f302de5 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -84,7 +84,10 @@ var { ClassComponent, } = require('ReactTypeOfWork'); -var {getUpdateExpirationTime} = require('ReactFiberUpdateQueue'); +var { + getUpdateExpirationTime, + insertUpdateIntoFiber, +} = require('ReactFiberUpdateQueue'); var {resetContext} = require('ReactFiberContext'); @@ -166,7 +169,6 @@ module.exports = function( hostContext, hydrationContext, scheduleUpdate, - getExpirationTime, ); const {completeWork} = ReactFiberCompleteWork( config, @@ -1373,11 +1375,33 @@ module.exports = function( } } - function scheduleUpdate(fiber: Fiber, expirationTime: ExpirationTime) { - return scheduleUpdateImpl(fiber, expirationTime, false); + function scheduleUpdate( + fiber: Fiber, + partialState: mixed, + callback: (() => mixed) | null, + isReplace: boolean, + isForced: boolean, + ) { + const expirationTime = getExpirationTime(fiber, false); + const update = { + expirationTime, + partialState, + callback, + isReplace, + isForced, + nextCallback: null, + isTopLevelUnmount: false, + next: null, + }; + insertUpdateIntoFiber(fiber, update); + scheduleWork(fiber, expirationTime); + } + + function scheduleWork(fiber: Fiber, expirationTime: ExpirationTime) { + return scheduleWorkImpl(fiber, expirationTime, false); } - function scheduleUpdateImpl( + function scheduleWorkImpl( fiber: Fiber, expirationTime: ExpirationTime, isErrorRecovery: boolean, @@ -1529,7 +1553,7 @@ module.exports = function( } function scheduleErrorRecovery(fiber: Fiber) { - scheduleUpdateImpl(fiber, Task, true); + scheduleWorkImpl(fiber, Task, true); } function recalculateCurrentTime(): ExpirationTime { @@ -1600,7 +1624,7 @@ module.exports = function( } return { - scheduleUpdate: scheduleUpdate, + scheduleWork: scheduleWork, getExpirationTime: getExpirationTime, batchedUpdates: batchedUpdates, unbatchedUpdates: unbatchedUpdates, diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 045e190efa77e..3006d237ffef6 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -19,7 +19,6 @@ const {NoWork} = require('ReactFiberExpirationTime'); const {ClassComponent, HostRoot} = require('ReactTypeOfWork'); -const invariant = require('fbjs/lib/invariant'); if (__DEV__) { var warning = require('fbjs/lib/warning'); } @@ -90,7 +89,7 @@ function cloneUpdate(update: Update): Update { }; } -function insertUpdateIntoQueue( +function insertUpdateIntoPosition( queue: UpdateQueue, update: Update, insertAfter: Update | null, @@ -187,7 +186,7 @@ function ensureUpdateQueues(fiber: Fiber) { // we shouldn't make a copy. // // If the update is cloned, it returns the cloned update. -function insertUpdate(fiber: Fiber, update: Update): Update | null { +function insertUpdateIntoFiber(fiber: Fiber, update: Update): Update | null { // We'll have at least one and at most two distinct update queues. ensureUpdateQueues(fiber); const queue1 = _queue1; @@ -214,7 +213,7 @@ function insertUpdate(fiber: Fiber, update: Update): Update | null { if (queue2 === null) { // If there's no alternate queue, there's nothing else to do but insert. - insertUpdateIntoQueue(queue1, update, insertAfter1, insertBefore1); + insertUpdateIntoPosition(queue1, update, insertAfter1, insertBefore1); return null; } @@ -226,7 +225,7 @@ function insertUpdate(fiber: Fiber, update: Update): Update | null { // Now we can insert into the first queue. This must come after finding both // insertion positions because it mutates the list. - insertUpdateIntoQueue(queue1, update, insertAfter1, insertBefore1); + insertUpdateIntoPosition(queue1, update, insertAfter1, insertBefore1); // See if the insertion positions are equal. Be careful to only compare // non-null values. @@ -249,66 +248,11 @@ function insertUpdate(fiber: Fiber, update: Update): Update | null { // The insertion positions are different, so we need to clone the update and // insert the clone into the alternate queue. const update2 = cloneUpdate(update); - insertUpdateIntoQueue(queue2, update2, insertAfter2, insertBefore2); + insertUpdateIntoPosition(queue2, update2, insertAfter2, insertBefore2); return update2; } } - -function addUpdate( - fiber: Fiber, - partialState: PartialState | null, - callback: mixed, - expirationTime: ExpirationTime, -): void { - const update = { - expirationTime, - partialState, - callback, - isReplace: false, - isForced: false, - isTopLevelUnmount: false, - next: null, - }; - insertUpdate(fiber, update); -} -exports.addUpdate = addUpdate; - -function addReplaceUpdate( - fiber: Fiber, - state: any | null, - callback: Callback | null, - expirationTime: ExpirationTime, -): void { - const update = { - expirationTime, - partialState: state, - callback, - isReplace: true, - isForced: false, - isTopLevelUnmount: false, - next: null, - }; - insertUpdate(fiber, update); -} -exports.addReplaceUpdate = addReplaceUpdate; - -function addForceUpdate( - fiber: Fiber, - callback: Callback | null, - expirationTime: ExpirationTime, -): void { - const update = { - expirationTime, - partialState: null, - callback, - isReplace: false, - isForced: true, - isTopLevelUnmount: false, - next: null, - }; - insertUpdate(fiber, update); -} -exports.addForceUpdate = addForceUpdate; +exports.insertUpdateIntoFiber = insertUpdateIntoFiber; function getUpdateExpirationTime(fiber: Fiber): ExpirationTime { const updateQueue = fiber.updateQueue; @@ -322,45 +266,6 @@ function getUpdateExpirationTime(fiber: Fiber): ExpirationTime { } exports.getUpdateExpirationTime = getUpdateExpirationTime; -function addTopLevelUpdate( - fiber: Fiber, - partialState: PartialState, - callback: Callback | null, - expirationTime: ExpirationTime, -): void { - const isTopLevelUnmount = partialState.element === null; - - const update = { - expirationTime, - partialState, - callback, - isReplace: false, - isForced: false, - isTopLevelUnmount, - next: null, - }; - const update2 = insertUpdate(fiber, update); - - if (isTopLevelUnmount) { - // TODO: Redesign the top-level mount/update/unmount API to avoid this - // special case. - const queue1 = _queue1; - const queue2 = _queue2; - - // Drop all updates that are lower-priority, so that the tree is not - // remounted. We need to do this for both queues. - if (queue1 !== null && update.next !== null) { - update.next = null; - queue1.last = update; - } - if (queue2 !== null && update2 !== null && update2.next !== null) { - update2.next = null; - queue2.last = update; - } - } -} -exports.addTopLevelUpdate = addTopLevelUpdate; - function getStateFromUpdate(update, instance, prevState, props) { const partialState = update.partialState; if (typeof partialState === 'function') { @@ -443,19 +348,20 @@ function beginUpdateQueue( ) { callbackList = callbackList !== null ? callbackList : []; callbackList.push(update.callback); - workInProgress.effectTag |= CallbackEffect; } update = update.next; } - queue.callbackList = callbackList; - queue.hasForceUpdate = hasForceUpdate; - - if (queue.first === null && callbackList === null && !hasForceUpdate) { - // The queue is empty and there are no callbacks. We can reset it. + if (callbackList !== null) { + workInProgress.effectTag |= CallbackEffect; + } else if (queue.first === null && !hasForceUpdate) { + // The queue is empty. We can reset it. workInProgress.updateQueue = null; } + queue.callbackList = callbackList; + queue.hasForceUpdate = hasForceUpdate; + if (__DEV__) { // No longer processing. queue.isProcessing = false; @@ -464,29 +370,3 @@ function beginUpdateQueue( return state; } exports.beginUpdateQueue = beginUpdateQueue; - -function commitCallbacks( - finishedWork: Fiber, - queue: UpdateQueue, - context: mixed, -) { - const callbackList = queue.callbackList; - if (callbackList === null) { - return; - } - - // Set the list to null to make sure they don't get called more than once. - queue.callbackList = null; - - for (let i = 0; i < callbackList.length; i++) { - const callback = callbackList[i]; - invariant( - typeof callback === 'function', - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: %s', - callback, - ); - callback.call(context); - } -} -exports.commitCallbacks = commitCallbacks; From 0f5f8302ce28bea4997286360706dde5fdfe20dc Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 14 Sep 2017 19:53:42 -0700 Subject: [PATCH 03/10] Deterministic updates High priority updates typically require less work to render than low priority ones. It's beneficial to flush those first, in their own batch, before working on more expensive low priority ones. We do this even if a high priority is scheduled after a low priority one. However, we don't want this reordering of updates to affect the terminal state. State should be deterministic: once all work has been flushed, the final state should be the same regardless of how they were scheduled. To get both properties, we store updates on the queue in insertion order instead of priority order (always append). Then, when processing the queue, we skip over updates with insufficient priority. Instead of removing updates from the queue right after processing them, we only remove them if there are no unprocessed updates before it in the list. This means that updates may be processed more than once. As a bonus, the new implementation is simpler and requires less code. --- package.json | 1 + scripts/jest/test-framework-setup.js | 2 + .../__tests__/ReactDOMFiberAsync-test.js | 4 +- src/renderers/noop/ReactNoopEntry.js | 2 +- src/renderers/shared/fiber/ReactFiber.js | 2 +- .../shared/fiber/ReactFiberBeginWork.js | 3 +- .../shared/fiber/ReactFiberClassComponent.js | 4 +- .../shared/fiber/ReactFiberCommitWork.js | 45 ++- .../shared/fiber/ReactFiberReconciler.js | 26 +- .../shared/fiber/ReactFiberUpdateQueue.js | 305 +++++++----------- .../ReactIncrementalScheduling-test.js | 8 +- .../ReactIncrementalTriangle-test.js | 1 + .../__tests__/ReactIncrementalUpdates-test.js | 30 +- yarn.lock | 10 + 14 files changed, 184 insertions(+), 259 deletions(-) diff --git a/package.json b/package.json index e042615c0232b..077bac9d161f1 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "glob-stream": "^6.1.0", "gzip-js": "~0.3.2", "gzip-size": "^3.0.0", + "jasmine-check": "^1.0.0-rc.0", "jest": "20.1.0-delta.1", "jest-config": "20.1.0-delta.1", "jest-jasmine2": "20.1.0-delta.1", diff --git a/scripts/jest/test-framework-setup.js b/scripts/jest/test-framework-setup.js index ca4706516710c..28ec0defae887 100644 --- a/scripts/jest/test-framework-setup.js +++ b/scripts/jest/test-framework-setup.js @@ -68,4 +68,6 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { return expectation; }; global.expectDev = expectDev; + + require('jasmine-check').install(); } diff --git a/src/renderers/dom/fiber/__tests__/ReactDOMFiberAsync-test.js b/src/renderers/dom/fiber/__tests__/ReactDOMFiberAsync-test.js index b432af4701a23..e8b4726e5c2f5 100644 --- a/src/renderers/dom/fiber/__tests__/ReactDOMFiberAsync-test.js +++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiberAsync-test.js @@ -284,8 +284,8 @@ describe('ReactDOMFiberAsync', () => { // Flush the async updates jest.runAllTimers(); - expect(container.textContent).toEqual('BCAD'); - expect(ops).toEqual(['BC', 'BCAD']); + expect(container.textContent).toEqual('ABCD'); + expect(ops).toEqual(['BC', 'ABCD']); }); }); }); diff --git a/src/renderers/noop/ReactNoopEntry.js b/src/renderers/noop/ReactNoopEntry.js index bf964eb84e86b..a1b7cf099a022 100644 --- a/src/renderers/noop/ReactNoopEntry.js +++ b/src/renderers/noop/ReactNoopEntry.js @@ -405,7 +405,7 @@ var ReactNoop = { logHostInstances(container.children, depth + 1); } - function logUpdateQueue(updateQueue: UpdateQueue, depth) { + function logUpdateQueue(updateQueue: UpdateQueue, depth) { log(' '.repeat(depth + 1) + 'QUEUED UPDATES'); const firstUpdate = updateQueue.first; if (!firstUpdate) { diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 3c81c35e31af3..7d03122297e74 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -109,7 +109,7 @@ export type Fiber = {| memoizedProps: any, // The props used to create the output. // A queue of state updates and callbacks. - updateQueue: UpdateQueue | null, + updateQueue: UpdateQueue | null, // The state used to create the output memoizedState: any, diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 272f72153eb81..7517dc1ce92ee 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -328,7 +328,6 @@ module.exports = function( workInProgress, updateQueue, null, - prevState, null, renderExpirationTime, ); @@ -338,7 +337,7 @@ module.exports = function( resetHydrationState(); return bailoutOnAlreadyFinishedWork(current, workInProgress); } - const element = state.element; + const element = state !== null ? state.element : null; if ( (current === null || current.child === null) && enterHydrationState(workInProgress) diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 6b7cf7f8812d5..6a135acfb8223 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -398,7 +398,7 @@ module.exports = function( const unmaskedContext = getUnmaskedContext(workInProgress); instance.props = props; - instance.state = state; + instance.state = workInProgress.memoizedState = state; instance.refs = emptyObject; instance.context = getMaskedContext(workInProgress, unmaskedContext); @@ -422,7 +422,6 @@ module.exports = function( workInProgress, updateQueue, instance, - state, props, renderExpirationTime, ); @@ -589,7 +588,6 @@ module.exports = function( workInProgress, workInProgress.updateQueue, instance, - oldState, newProps, renderExpirationTime, ); diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index e67f17bac2f5c..6b04e36a39196 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -29,12 +29,7 @@ var { clearCaughtError, } = require('ReactErrorUtils'); -var { - Placement, - Update, - Callback, - ContentReset, -} = require('ReactTypeOfSideEffect'); +var {Placement, Update, ContentReset} = require('ReactTypeOfSideEffect'); var invariant = require('fbjs/lib/invariant'); @@ -102,9 +97,16 @@ module.exports = function( } } - function commitCallbacks(callbackList, context) { - for (let i = 0; i < callbackList.length; i++) { - const callback = callbackList[i]; + function commitCallbacks(updateQueue, context) { + let callbackNode = updateQueue.firstCallback; + // Reset the callback list before calling them in case something throws. + updateQueue.firstCallback = updateQueue.lastCallback = null; + + while (callbackNode !== null) { + const callback = callbackNode.callback; + // Remove this callback from the update object in case it's still part + // of the queue, so that we don't call it again. + callbackNode.callback = null; invariant( typeof callback === 'function', 'Invalid argument passed as callback. Expected a function. Instead ' + @@ -112,6 +114,9 @@ module.exports = function( callback, ); callback.call(context); + const nextCallback = callbackNode.nextCallback; + callbackNode.nextCallback = null; + callbackNode = nextCallback; } } @@ -144,31 +149,19 @@ module.exports = function( } } } - if ( - finishedWork.effectTag & Callback && - finishedWork.updateQueue !== null - ) { - const updateQueue = finishedWork.updateQueue; - if (updateQueue.callbackList !== null) { - // Set the list to null to make sure they don't get called more than once. - const callbackList = updateQueue.callbackList; - updateQueue.callbackList = null; - commitCallbacks(callbackList, instance); - } + const updateQueue = finishedWork.updateQueue; + if (updateQueue !== null) { + commitCallbacks(updateQueue, instance); } return; } case HostRoot: { const updateQueue = finishedWork.updateQueue; - if (updateQueue !== null && updateQueue.callbackList !== null) { - // Set the list to null to make sure they don't get called more - // than once. - const callbackList = updateQueue.callbackList; - updateQueue.callbackList = null; + if (updateQueue !== null) { const instance = finishedWork.child !== null ? finishedWork.child.stateNode : null; - commitCallbacks(callbackList, instance); + commitCallbacks(updateQueue, instance); } return; } diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index a9d84bd9ddc75..10bd72be272db 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -316,38 +316,16 @@ module.exports = function( callback, ); } - const isTopLevelUnmount = nextState.element === null; const update = { expirationTime, partialState: nextState, callback, isReplace: false, isForced: false, - isTopLevelUnmount, + nextCallback: null, next: null, }; - const update2 = insertUpdateIntoFiber(current, update); - - if (isTopLevelUnmount) { - // TODO: Redesign the top-level mount/update/unmount API to avoid this - // special case. - const queue1 = current.updateQueue; - const queue2 = current.alternate !== null - ? current.alternate.updateQueue - : null; - - // Drop all updates that are lower-priority, so that the tree is not - // remounted. We need to do this for both queues. - if (queue1 !== null && update.next !== null) { - update.next = null; - queue1.last = update; - } - if (queue2 !== null && update2 !== null && update2.next !== null) { - update2.next = null; - queue2.last = update; - } - } - + insertUpdateIntoFiber(current, update); scheduleWork(current, expirationTime); } diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 3006d237ffef6..f23000674e414 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -30,14 +30,14 @@ type PartialState = // Callbacks are not validated until invocation type Callback = mixed; -export type Update = { +export type Update = { expirationTime: ExpirationTime, partialState: PartialState, callback: Callback | null, isReplace: boolean, isForced: boolean, - isTopLevelUnmount: boolean, - next: Update | null, + next: Update | null, + nextCallback: Update | null, }; // Singly linked-list of updates. When an update is scheduled, it is added to @@ -51,25 +51,33 @@ export type Update = { // The work-in-progress queue is always a subset of the current queue. // // When the tree is committed, the work-in-progress becomes the current. -export type UpdateQueue = { - first: Update | null, - last: Update | null, +export type UpdateQueue = { + // A processed update is not removed from the queue if there are any + // unprocessed updates that came before it. In that case, we need to keep + // track of the base state, which represents the base state of the first + // unprocessed update, which is the same as the first update in the list. + baseState: State | null, + // For the same reason, we keep track of the remaining expiration time. + expirationTime: ExpirationTime, + first: Update | null, + last: Update | null, + firstCallback: Update | null, + lastCallback: Update | null, hasForceUpdate: boolean, - callbackList: null | Array, // Dev only isProcessing?: boolean, }; -let _queue1; -let _queue2; - -function createUpdateQueue(): UpdateQueue { - const queue: UpdateQueue = { +function createUpdateQueue(baseState: State): UpdateQueue { + const queue: UpdateQueue = { + baseState, + expirationTime: NoWork, first: null, last: null, + firstCallback: null, + lastCallback: null, hasForceUpdate: false, - callbackList: null, }; if (__DEV__) { queue.isProcessing = false; @@ -77,120 +85,47 @@ function createUpdateQueue(): UpdateQueue { return queue; } -function cloneUpdate(update: Update): Update { - return { - expirationTime: update.expirationTime, - partialState: update.partialState, - callback: update.callback, - isReplace: update.isReplace, - isForced: update.isForced, - isTopLevelUnmount: update.isTopLevelUnmount, - next: null, - }; -} - -function insertUpdateIntoPosition( - queue: UpdateQueue, - update: Update, - insertAfter: Update | null, - insertBefore: Update | null, +function insertUpdateIntoQueue( + queue: UpdateQueue, + update: Update, ) { - if (insertAfter !== null) { - insertAfter.next = update; - } else { - // This is the first item in the queue. - update.next = queue.first; - queue.first = update; - } - - if (insertBefore !== null) { - update.next = insertBefore; + // Append the update to the end of the list. + if (queue.last === null) { + // Queue is empty + queue.first = queue.last = update; } else { - // This is the last item in the queue. + queue.last.next = update; queue.last = update; } -} - -// Returns the update after which the incoming update should be inserted into -// the queue, or null if it should be inserted at beginning. -function findInsertionPosition(queue, update): Update | null { - const expirationTime = update.expirationTime; - let insertAfter = null; - let insertBefore = null; - if (queue.last !== null && queue.last.expirationTime <= expirationTime) { - // 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 !== null && - insertBefore.expirationTime <= expirationTime - ) { - insertAfter = insertBefore; - insertBefore = insertBefore.next; - } + if ( + queue.expirationTime === NoWork || + queue.expirationTime > update.expirationTime + ) { + queue.expirationTime = update.expirationTime; } - return insertAfter; } +exports.insertUpdateIntoQueue = insertUpdateIntoQueue; -function ensureUpdateQueues(fiber: Fiber) { +function insertUpdateIntoFiber(fiber: Fiber, update: Update) { + // We'll have at least one and at most two distinct update queues. const alternateFiber = fiber.alternate; - let queue1 = fiber.updateQueue; if (queue1 === null) { - queue1 = fiber.updateQueue = createUpdateQueue(); + queue1 = fiber.updateQueue = createUpdateQueue(fiber.memoizedState); } let queue2; if (alternateFiber !== null) { queue2 = alternateFiber.updateQueue; if (queue2 === null) { - queue2 = alternateFiber.updateQueue = createUpdateQueue(); + queue2 = alternateFiber.updateQueue = createUpdateQueue( + alternateFiber.memoizedState, + ); } } else { queue2 = null; } - - _queue1 = queue1; - // Return null if there is no alternate queue, or if its queue is the same. - _queue2 = queue2 !== queue1 ? queue2 : null; -} - -// The work-in-progress queue is a subset of the current queue (if it exists). -// We need to insert the incoming update into both lists. However, it's possible -// that the correct position in one list will be different from the position in -// the other. Consider the following case: -// -// Current: 3-5-6 -// Work-in-progress: 6 -// -// Then we receive an update with priority 4 and insert it into each list: -// -// Current: 3-4-5-6 -// Work-in-progress: 4-6 -// -// In the current queue, the new update's `next` pointer points to the update -// with priority 5. But in the work-in-progress queue, the pointer points to the -// update with priority 6. Because these two queues share the same persistent -// data structure, this won't do. (This can only happen when the incoming update -// has higher priority than all the updates in the work-in-progress queue.) -// -// To solve this, in the case where the incoming update needs to be inserted -// into two different positions, we'll make a clone of the update and insert -// each copy into a separate queue. This forks the list while maintaining a -// persistent structure, because the update that is added to the work-in-progress -// is always added to the front of the list. -// -// However, if incoming update is inserted into the same position of both lists, -// we shouldn't make a copy. -// -// If the update is cloned, it returns the cloned update. -function insertUpdateIntoFiber(fiber: Fiber, update: Update): Update | null { - // We'll have at least one and at most two distinct update queues. - ensureUpdateQueues(fiber); - const queue1 = _queue1; - const queue2 = _queue2; + queue2 = queue2 !== queue1 ? queue2 : null; // Warn if an update is scheduled from inside an updater function. if (__DEV__) { @@ -205,64 +140,37 @@ function insertUpdateIntoFiber(fiber: Fiber, update: Update): Update | null { } } - // Find the insertion position in the first queue. - const insertAfter1 = findInsertionPosition(queue1, update); - const insertBefore1 = insertAfter1 !== null - ? insertAfter1.next - : queue1.first; - + // If there's only one queue, add the update to that queue and exit. if (queue2 === null) { - // If there's no alternate queue, there's nothing else to do but insert. - insertUpdateIntoPosition(queue1, update, insertAfter1, insertBefore1); - return null; + insertUpdateIntoQueue(queue1, update); + return; } - // If there is an alternate queue, find the insertion position. - const insertAfter2 = findInsertionPosition(queue2, update); - const insertBefore2 = insertAfter2 !== null - ? insertAfter2.next - : queue2.first; - - // Now we can insert into the first queue. This must come after finding both - // insertion positions because it mutates the list. - insertUpdateIntoPosition(queue1, update, insertAfter1, insertBefore1); - - // See if the insertion positions are equal. Be careful to only compare - // non-null values. - if ( - (insertBefore1 === insertBefore2 && insertBefore1 !== null) || - (insertAfter1 === insertAfter2 && insertAfter1 !== null) - ) { - // The insertion positions are the same, so when we inserted into the first - // queue, it also inserted into the alternate. All we need to do is update - // the alternate queue's `first` and `last` pointers, in case they - // have changed. - if (insertAfter2 === null) { - queue2.first = update; - } - if (insertBefore2 === null) { - queue2.last = null; - } - return null; - } else { - // The insertion positions are different, so we need to clone the update and - // insert the clone into the alternate queue. - const update2 = cloneUpdate(update); - insertUpdateIntoPosition(queue2, update2, insertAfter2, insertBefore2); - return update2; + // If either queue is empty, we need to add to both queues. + if (queue1.last === null || queue2.last === null) { + insertUpdateIntoQueue(queue1, update); + insertUpdateIntoQueue(queue2, update); + return; } + + // If both lists are not empty, the last update is the same for both lists + // because of structural sharing. So, we should only append to one of + // the lists. + insertUpdateIntoQueue(queue1, update); + // But we still need to update the `last` pointer of queue2. + queue2.last = update; } exports.insertUpdateIntoFiber = insertUpdateIntoFiber; function getUpdateExpirationTime(fiber: Fiber): ExpirationTime { - const updateQueue = fiber.updateQueue; - if (updateQueue === null) { + if (fiber.tag !== ClassComponent && fiber.tag !== HostRoot) { return NoWork; } - if (fiber.tag !== ClassComponent && fiber.tag !== HostRoot) { + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { return NoWork; } - return updateQueue.first !== null ? updateQueue.first.expirationTime : NoWork; + return updateQueue.expirationTime; } exports.getUpdateExpirationTime = getUpdateExpirationTime; @@ -276,12 +184,11 @@ function getStateFromUpdate(update, instance, prevState, props) { } } -function beginUpdateQueue( +function beginUpdateQueue( current: Fiber | null, workInProgress: Fiber, - queue: UpdateQueue, + queue: UpdateQueue, instance: any, - prevState: any, props: any, renderExpirationTime: ExpirationTime, ): any { @@ -289,11 +196,14 @@ function beginUpdateQueue( // We need to create a work-in-progress queue, by cloning the current queue. const currentQueue = queue; queue = workInProgress.updateQueue = { + baseState: currentQueue.baseState, + expirationTime: currentQueue.expirationTime, first: currentQueue.first, last: currentQueue.last, // These fields are no longer valid because they were already committed. // Reset them. - callbackList: null, + firstCallback: null, + lastCallback: null, hasForceUpdate: false, }; } @@ -304,24 +214,50 @@ function beginUpdateQueue( queue.isProcessing = true; } - // Calculate these using the the existing values as a base. - let callbackList = queue.callbackList; - let hasForceUpdate = queue.hasForceUpdate; + // Reset the remaining expiration time. If we skip over any updates, we'll + // increase this accordingly. + queue.expirationTime = NoWork; - // Applies updates with matching priority to the previous state to create - // a new state object. - let state = prevState; + let state = queue.baseState; let dontMutatePrevState = true; let update = queue.first; - while (update !== null && update.expirationTime <= renderExpirationTime) { - // 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 === null) { - queue.last = null; + let didSkip = false; + while (update !== null) { + const updateExpirationTime = update.expirationTime; + if ( + updateExpirationTime === NoWork || + updateExpirationTime > renderExpirationTime + ) { + // This update does not have sufficient priority. Skip it. + const remainingExpirationTime = queue.expirationTime; + if ( + remainingExpirationTime === NoWork || + remainingExpirationTime > updateExpirationTime + ) { + // Update the remaining expiration time. + queue.expirationTime = updateExpirationTime; + } + if (!didSkip) { + didSkip = true; + queue.baseState = state; + } + // Continue to the next update. + update = update.next; + continue; + } + + // This update does have sufficient priority. + + // If no previous updates were skipped, drop this update from the queue by + // advancing the head of the list. + if (!didSkip) { + queue.first = update.next; + if (queue.first === null) { + queue.last = null; + } } + // Process the update let partialState; if (update.isReplace) { state = getStateFromUpdate(update, instance, state, props); @@ -330,6 +266,7 @@ function beginUpdateQueue( partialState = getStateFromUpdate(update, instance, state, props); if (partialState) { if (dontMutatePrevState) { + // $FlowFixMe: Idk how to type this properly. state = Object.assign({}, state, partialState); } else { state = Object.assign(state, partialState); @@ -338,29 +275,31 @@ function beginUpdateQueue( } } if (update.isForced) { - hasForceUpdate = true; + queue.hasForceUpdate = true; } - // Second condition ignores top-level unmount callbacks if they are not the - // last update in the queue, since a subsequent update will cause a remount. - if ( - update.callback !== null && - !(update.isTopLevelUnmount && update.next !== null) - ) { - callbackList = callbackList !== null ? callbackList : []; - callbackList.push(update.callback); + if (update.callback !== null) { + // Append to list of callbacks. + if (queue.lastCallback === null) { + queue.lastCallback = queue.firstCallback = update; + } else { + queue.lastCallback.nextCallback = update; + queue.lastCallback = update; + } } update = update.next; } - if (callbackList !== null) { + if (queue.lastCallback !== null) { workInProgress.effectTag |= CallbackEffect; - } else if (queue.first === null && !hasForceUpdate) { + } else if (queue.first === null && !queue.hasForceUpdate) { // The queue is empty. We can reset it. workInProgress.updateQueue = null; } - queue.callbackList = callbackList; - queue.hasForceUpdate = hasForceUpdate; + if (!didSkip) { + didSkip = true; + queue.baseState = state; + } if (__DEV__) { // No longer processing. diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js index 3d44622333097..1569f2bc98c42 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js @@ -57,11 +57,13 @@ describe('ReactIncrementalScheduling', () => { ReactNoop.render(); }); }); + // The sync updates flush first. + expect(ReactNoop.getChildren()).toEqual([span(4)]); - // The low pri update should be flushed last, even though it was scheduled - // before the sync updates. + // The terminal value should be the last update that was scheduled, + // regardless of priority. In this case, that's the last sync update. ReactNoop.flush(); - expect(ReactNoop.getChildren()).toEqual([span(5)]); + expect(ReactNoop.getChildren()).toEqual([span(4)]); }); it('schedules top-level updates with same priority in order of insertion', () => { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalTriangle-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalTriangle-test.js index 7acffd8991c54..bec1f128974b1 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalTriangle-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalTriangle-test.js @@ -246,6 +246,7 @@ describe('ReactIncrementalTriangle', () => { simulate(step(1), flush(3), toggle(18), step(0)); simulate(step(4), flush(52), expire(1476), flush(17), step(0)); simulate(interrupt(), toggle(10), step(2), expire(990), flush(46)); + simulate(interrupt(), step(6), step(7), toggle(6), interrupt()); }); it('fuzz tester', () => { diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js index 224cb0e2ddb74..0d00233462f4d 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js @@ -154,19 +154,20 @@ describe('ReactIncrementalUpdates', () => { expect(ReactNoop.getChildren()).toEqual([span('')]); // Schedule some more updates at different priorities{ - instance.setState(createUpdate('f')); + instance.setState(createUpdate('d')); ReactNoop.flushSync(() => { - instance.setState(createUpdate('d')); instance.setState(createUpdate('e')); + instance.setState(createUpdate('f')); }); instance.setState(createUpdate('g')); // The sync updates should have flushed, but not the async ones - expect(ReactNoop.getChildren()).toEqual([span('de')]); + expect(ReactNoop.getChildren()).toEqual([span('ef')]); - // Now flush the remaining work. Updates a, b, and c should be processed - // again, since they were interrupted last time. - expect(ReactNoop.flush()).toEqual(['a', 'b', 'c', 'f', 'g']); + // Now flush the remaining work. Even though e and f were already processed, + // they should be processed again, to ensure that the terminal state + // is deterministic. + expect(ReactNoop.flush()).toEqual(['a', 'b', 'c', 'd', 'e', 'f', 'g']); expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]); }); @@ -202,23 +203,24 @@ describe('ReactIncrementalUpdates', () => { expect(ReactNoop.getChildren()).toEqual([span('')]); // Schedule some more updates at different priorities{ - instance.setState(createUpdate('f')); + instance.setState(createUpdate('d')); ReactNoop.flushSync(() => { - instance.setState(createUpdate('d')); + instance.setState(createUpdate('e')); // No longer a public API, but we can test that it works internally by // reaching into the updater. - instance.updater.enqueueReplaceState(instance, createUpdate('e')); + instance.updater.enqueueReplaceState(instance, createUpdate('f')); }); instance.setState(createUpdate('g')); // The sync updates should have flushed, but not the async ones. Update d // was dropped and replaced by e. - expect(ReactNoop.getChildren()).toEqual([span('e')]); + expect(ReactNoop.getChildren()).toEqual([span('f')]); - // Now flush the remaining work. Updates a, b, and c should be processed - // again, since they were interrupted last time. - expect(ReactNoop.flush()).toEqual(['a', 'b', 'c', 'f', 'g']); - expect(ReactNoop.getChildren()).toEqual([span('abcefg')]); + // Now flush the remaining work. Even though e and f were already processed, + // they should be processed again, to ensure that the terminal state + // is deterministic. + expect(ReactNoop.flush()).toEqual(['a', 'b', 'c', 'd', 'e', 'f', 'g']); + expect(ReactNoop.getChildren()).toEqual([span('fg')]); }); it('passes accumulation of previous updates to replaceState updater function', () => { diff --git a/yarn.lock b/yarn.lock index 76c2c27615f09..4cd5cfb7290c3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2614,6 +2614,12 @@ istanbul-reports@^1.1.1: dependencies: handlebars "^4.0.3" +jasmine-check@^1.0.0-rc.0: + version "1.0.0-rc.0" + resolved "https://registry.yarnpkg.com/jasmine-check/-/jasmine-check-1.0.0-rc.0.tgz#117728c150078ecf211986c5f164275b71e937a4" + dependencies: + testcheck "^1.0.0-rc" + jest-changed-files@20.1.0-delta.1: version "20.1.0-delta.1" resolved "https://registry.yarnpkg.com/jest-changed-files/-/jest-changed-files-20.1.0-delta.1.tgz#912f8eff09c79b28fc7b66513f0ad505f1b378d5" @@ -4316,6 +4322,10 @@ test-exclude@^4.0.3: read-pkg-up "^1.0.1" require-main-filename "^1.0.1" +testcheck@^1.0.0-rc: + version "1.0.0-rc.2" + resolved "https://registry.yarnpkg.com/testcheck/-/testcheck-1.0.0-rc.2.tgz#11356a25b84575efe0b0857451e85b5fa74ee4e4" + text-table@~0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/text-table/-/text-table-0.2.0.tgz#7f5ee823ae805207c00af2df4a84ec3fcfa570b4" From 3b8ef0e9ad79f2e94904f84a8c5e0d947f5be880 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 11 Oct 2017 18:43:29 -0700 Subject: [PATCH 04/10] Remove getExpirationTime The last remaining use for getExpirationTime was for top-level async updates. I moved that check to scheduleUpdate instead. --- .../shared/fiber/ReactFiberReconciler.js | 31 +----- .../shared/fiber/ReactFiberScheduler.js | 104 ++++++++++-------- 2 files changed, 59 insertions(+), 76 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 10bd72be272db..44ecb135e50f0 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -14,10 +14,6 @@ import type {Fiber} from 'ReactFiber'; import type {FiberRoot} from 'ReactFiberRoot'; import type {ReactNodeList} from 'ReactTypes'; -var ReactFeatureFlags = require('ReactFeatureFlags'); - -var {insertUpdateIntoFiber} = require('ReactFiberUpdateQueue'); - var { findCurrentUnmaskedContext, isContextProvider, @@ -265,8 +261,7 @@ module.exports = function( var {getPublicInstance} = config; var { - scheduleWork, - getExpirationTime, + scheduleUpdate, batchedUpdates, unbatchedUpdates, flushSync, @@ -296,17 +291,6 @@ module.exports = function( } } - // Check if the top-level element is an async wrapper component. If so, treat - // updates to the root as async. This is a bit weird but lets us avoid a separate - // `renderAsync` API. - const forceAsync = - ReactFeatureFlags.enableAsyncSubtreeAPI && - element != null && - element.type != null && - element.type.prototype != null && - (element.type.prototype: any).unstable_isAsyncReactComponent === true; - const expirationTime = getExpirationTime(current, forceAsync); - const nextState = {element}; callback = callback === undefined ? null : callback; if (__DEV__) { warning( @@ -316,17 +300,8 @@ module.exports = function( callback, ); } - const update = { - expirationTime, - partialState: nextState, - callback, - isReplace: false, - isForced: false, - nextCallback: null, - next: null, - }; - insertUpdateIntoFiber(current, update); - scheduleWork(current, expirationTime); + const state = {element}; + scheduleUpdate(current, state, callback, false, false); } return { diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 8c81c9f302de5..ad1d94538a57b 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -30,6 +30,7 @@ export type HandleErrorInfo = { componentStack: string, }; +var ReactFeatureFlags = require('ReactFeatureFlags'); var {popContextProvider} = require('ReactFiberContext'); const {reset} = require('ReactFiberStack'); var { @@ -1382,7 +1383,60 @@ module.exports = function( isReplace: boolean, isForced: boolean, ) { - const expirationTime = getExpirationTime(fiber, false); + let expirationTime; + if (expirationContext !== NoWork) { + // An explicit expiration context was set; + expirationTime = expirationContext; + } else if (isPerformingWork) { + if (isCommitting) { + // Updates that occur during the commit phase should have sync priority + // by default. + expirationTime = Sync; + } else { + // Updates during the render phase should expire at the same time as + // the work that is being rendered. + expirationTime = nextRenderExpirationTime; + } + } else { + // No explicit expiration context was set, and we're not currently + // performing work. Calculate a new expiration time. + + let forceAsync = false; + if (fiber.tag === HostRoot) { + // Check if the top-level element is an async wrapper component. If so, + // treat updates to the root as async. This is a bit weird but lets us + // avoid a separate `renderAsync` API. + const element = (partialState: any).element; + forceAsync = + ReactFeatureFlags.enableAsyncSubtreeAPI && + element != null && + element.type != null && + element.type.prototype != null && + (element.type.prototype: any).unstable_isAsyncReactComponent === true; + } + + if ( + useSyncScheduling && + !(fiber.internalContextTag & AsyncUpdates) && + !forceAsync + ) { + // This is a sync update + expirationTime = Sync; + } else { + // This is an async update + const currentTime = recalculateCurrentTime(); + expirationTime = asyncExpirationTime(currentTime); + } + } + + if ( + expirationTime === Sync && + (isBatchingUpdates || (isUnbatchingUpdates && isCommitting)) + ) { + // If we're in a batch, downgrade sync to task. + expirationTime = Task; + } + const update = { expirationTime, partialState, @@ -1507,51 +1561,6 @@ module.exports = function( } } - function getExpirationTime( - fiber: Fiber, - forceAsync: boolean, - ): ExpirationTime { - let expirationTime; - if (expirationContext !== NoWork) { - // An explicit expiration context was set; - expirationTime = expirationContext; - } else if (isPerformingWork) { - if (isCommitting) { - // Updates that occur during the commit phase should have task priority - // by default. - expirationTime = Sync; - } else { - // Updates during the render phase should expire at the same time as - // the work that is being rendered. - expirationTime = nextRenderExpirationTime; - } - } else { - // No explicit expiration context was set, and we're not currently - // performing work. Calculate a new expiration time. - if ( - useSyncScheduling && - !(fiber.internalContextTag & AsyncUpdates) && - !forceAsync - ) { - // This is a sync update - expirationTime = Sync; - } else { - // This is an async update - const currentTime = recalculateCurrentTime(); - expirationTime = asyncExpirationTime(currentTime); - } - } - - if ( - expirationTime === Sync && - (isBatchingUpdates || (isUnbatchingUpdates && isCommitting)) - ) { - // If we're in a batch, or in the commit phase, downgrade sync to task - return Task; - } - return expirationTime; - } - function scheduleErrorRecovery(fiber: Fiber) { scheduleWorkImpl(fiber, Task, true); } @@ -1624,8 +1633,7 @@ module.exports = function( } return { - scheduleWork: scheduleWork, - getExpirationTime: getExpirationTime, + scheduleUpdate: scheduleUpdate, batchedUpdates: batchedUpdates, unbatchedUpdates: unbatchedUpdates, flushSync: flushSync, From 7ce4ae1945b988292bbf5f860e4d31edac0be156 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 13 Oct 2017 14:57:01 -0700 Subject: [PATCH 05/10] Move UpdateQueue insertions back to class module Moves UpdateQueue related functions out of the scheduler and back into the class component module. It's a bit awkward that now we need to pass around createUpdateExpirationForFiber, too. But we can still do without addUpdate, replaceUpdate, et al. --- .../shared/fiber/ReactFiberBeginWork.js | 16 +++--- .../shared/fiber/ReactFiberClassComponent.js | 53 ++++++++++++++---- .../shared/fiber/ReactFiberReconciler.js | 38 ++++++++++++- .../shared/fiber/ReactFiberScheduler.js | 56 +++---------------- 4 files changed, 94 insertions(+), 69 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 7517dc1ce92ee..ed8f6af306790 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -71,13 +71,8 @@ module.exports = function( config: HostConfig, hostContext: HostContext, hydrationContext: HydrationContext, - scheduleUpdate: ( - fiber: Fiber, - partialState: mixed, - callback: (() => mixed) | null, - isReplace: boolean, - isForced: boolean, - ) => void, + scheduleWork: (fiber: Fiber, expirationTime: ExpirationTime) => void, + createUpdateExpirationForFiber: (fiber: Fiber) => ExpirationTime, ) { const { shouldSetTextContent, @@ -99,7 +94,12 @@ module.exports = function( mountClassInstance, // resumeMountClassInstance, updateClassInstance, - } = ReactFiberClassComponent(scheduleUpdate, memoizeProps, memoizeState); + } = ReactFiberClassComponent( + scheduleWork, + createUpdateExpirationForFiber, + memoizeProps, + memoizeState, + ); // TODO: Remove this and use reconcileChildrenAtExpirationTime directly. function reconcileChildren(current, workInProgress, nextChildren) { diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 6a135acfb8223..a428d27e6ac0c 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -24,7 +24,10 @@ var { getUnmaskedContext, isContextConsumer, } = require('ReactFiberContext'); -var {beginUpdateQueue} = require('ReactFiberUpdateQueue'); +var { + insertUpdateIntoFiber, + beginUpdateQueue, +} = require('ReactFiberUpdateQueue'); var {hasContextChanged} = require('ReactFiberContext'); var {isMounted} = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); @@ -72,13 +75,8 @@ if (__DEV__) { } module.exports = function( - scheduleUpdate: ( - fiber: Fiber, - partialState: mixed, - callback: (() => mixed) | null, - isReplace: boolean, - isForced: boolean, - ) => void, + scheduleWork: (fiber: Fiber, expirationTime: ExpirationTime) => void, + createUpdateExpirationForFiber: (fiber: Fiber) => ExpirationTime, memoizeProps: (workInProgress: Fiber, props: any) => void, memoizeState: (workInProgress: Fiber, state: any) => void, ) { @@ -91,7 +89,18 @@ module.exports = function( if (__DEV__) { warnOnInvalidCallback(callback, 'setState'); } - scheduleUpdate(fiber, partialState, callback, false, false); + const expirationTime = createUpdateExpirationForFiber(fiber); + const update = { + expirationTime, + partialState, + callback, + isReplace: false, + isForced: false, + nextCallback: null, + next: null, + }; + insertUpdateIntoFiber(fiber, update); + scheduleWork(fiber, expirationTime); }, enqueueReplaceState(instance, state, callback) { const fiber = ReactInstanceMap.get(instance); @@ -99,7 +108,18 @@ module.exports = function( if (__DEV__) { warnOnInvalidCallback(callback, 'replaceState'); } - scheduleUpdate(fiber, state, callback, true, false); + const expirationTime = createUpdateExpirationForFiber(fiber); + const update = { + expirationTime, + partialState: state, + callback, + isReplace: true, + isForced: false, + nextCallback: null, + next: null, + }; + insertUpdateIntoFiber(fiber, update); + scheduleWork(fiber, expirationTime); }, enqueueForceUpdate(instance, callback) { const fiber = ReactInstanceMap.get(instance); @@ -107,7 +127,18 @@ module.exports = function( if (__DEV__) { warnOnInvalidCallback(callback, 'forceUpdate'); } - scheduleUpdate(fiber, null, callback, false, true); + const expirationTime = createUpdateExpirationForFiber(fiber); + const update = { + expirationTime, + partialState: null, + callback, + isReplace: false, + isForced: true, + nextCallback: null, + next: null, + }; + insertUpdateIntoFiber(fiber, update); + scheduleWork(fiber, expirationTime); }, }; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 44ecb135e50f0..6d0efb6c54798 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -14,6 +14,7 @@ import type {Fiber} from 'ReactFiber'; import type {FiberRoot} from 'ReactFiberRoot'; import type {ReactNodeList} from 'ReactTypes'; +var ReactFeatureFlags = require('ReactFeatureFlags'); var { findCurrentUnmaskedContext, isContextProvider, @@ -23,6 +24,8 @@ var {createFiberRoot} = require('ReactFiberRoot'); var ReactFiberScheduler = require('ReactFiberScheduler'); var ReactInstanceMap = require('ReactInstanceMap'); var {HostComponent} = require('ReactTypeOfWork'); +var {asyncExpirationTime} = require('ReactFiberExpirationTime'); +var {insertUpdateIntoFiber} = require('ReactFiberUpdateQueue'); var emptyObject = require('fbjs/lib/emptyObject'); if (__DEV__) { @@ -261,7 +264,9 @@ module.exports = function( var {getPublicInstance} = config; var { - scheduleUpdate, + createUpdateExpirationForFiber, + recalculateCurrentTime, + scheduleWork, batchedUpdates, unbatchedUpdates, flushSync, @@ -300,8 +305,35 @@ module.exports = function( callback, ); } - const state = {element}; - scheduleUpdate(current, state, callback, false, false); + + let expirationTime; + // Check if the top-level element is an async wrapper component. If so, + // treat updates to the root as async. This is a bit weird but lets us + // avoid a separate `renderAsync` API. + if ( + ReactFeatureFlags.enableAsyncSubtreeAPI && + element != null && + element.type != null && + element.type.prototype != null && + (element.type.prototype: any).unstable_isAsyncReactComponent === true + ) { + const currentTime = recalculateCurrentTime(); + expirationTime = asyncExpirationTime(currentTime); + } else { + expirationTime = createUpdateExpirationForFiber(current); + } + + const update = { + expirationTime, + partialState: {element}, + callback, + isReplace: false, + isForced: false, + nextCallback: null, + next: null, + }; + insertUpdateIntoFiber(current, update); + scheduleWork(current, expirationTime); } return { diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index ad1d94538a57b..315c04101cbda 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -30,7 +30,6 @@ export type HandleErrorInfo = { componentStack: string, }; -var ReactFeatureFlags = require('ReactFeatureFlags'); var {popContextProvider} = require('ReactFiberContext'); const {reset} = require('ReactFiberStack'); var { @@ -85,10 +84,7 @@ var { ClassComponent, } = require('ReactTypeOfWork'); -var { - getUpdateExpirationTime, - insertUpdateIntoFiber, -} = require('ReactFiberUpdateQueue'); +var {getUpdateExpirationTime} = require('ReactFiberUpdateQueue'); var {resetContext} = require('ReactFiberContext'); @@ -169,7 +165,8 @@ module.exports = function( config, hostContext, hydrationContext, - scheduleUpdate, + scheduleWork, + createUpdateExpirationForFiber, ); const {completeWork} = ReactFiberCompleteWork( config, @@ -1376,13 +1373,7 @@ module.exports = function( } } - function scheduleUpdate( - fiber: Fiber, - partialState: mixed, - callback: (() => mixed) | null, - isReplace: boolean, - isForced: boolean, - ) { + function createUpdateExpirationForFiber(fiber: Fiber) { let expirationTime; if (expirationContext !== NoWork) { // An explicit expiration context was set; @@ -1400,26 +1391,7 @@ module.exports = function( } else { // No explicit expiration context was set, and we're not currently // performing work. Calculate a new expiration time. - - let forceAsync = false; - if (fiber.tag === HostRoot) { - // Check if the top-level element is an async wrapper component. If so, - // treat updates to the root as async. This is a bit weird but lets us - // avoid a separate `renderAsync` API. - const element = (partialState: any).element; - forceAsync = - ReactFeatureFlags.enableAsyncSubtreeAPI && - element != null && - element.type != null && - element.type.prototype != null && - (element.type.prototype: any).unstable_isAsyncReactComponent === true; - } - - if ( - useSyncScheduling && - !(fiber.internalContextTag & AsyncUpdates) && - !forceAsync - ) { + if (useSyncScheduling && !(fiber.internalContextTag & AsyncUpdates)) { // This is a sync update expirationTime = Sync; } else { @@ -1436,19 +1408,7 @@ module.exports = function( // If we're in a batch, downgrade sync to task. expirationTime = Task; } - - const update = { - expirationTime, - partialState, - callback, - isReplace, - isForced, - nextCallback: null, - isTopLevelUnmount: false, - next: null, - }; - insertUpdateIntoFiber(fiber, update); - scheduleWork(fiber, expirationTime); + return expirationTime; } function scheduleWork(fiber: Fiber, expirationTime: ExpirationTime) { @@ -1633,7 +1593,9 @@ module.exports = function( } return { - scheduleUpdate: scheduleUpdate, + recalculateCurrentTime: recalculateCurrentTime, + createUpdateExpirationForFiber: createUpdateExpirationForFiber, + scheduleWork: scheduleWork, batchedUpdates: batchedUpdates, unbatchedUpdates: unbatchedUpdates, flushSync: flushSync, From c071372200f482b221e7e6fddc604fa38885e1c3 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 13 Oct 2017 15:11:03 -0700 Subject: [PATCH 06/10] Store callbacks as an array of Updates Simpler this way. Also moves commitCallbacks back to UpdateQueue module. --- src/renderers/shared/fiber/ReactFiber.js | 2 +- .../shared/fiber/ReactFiberCommitWork.js | 24 +--------- .../shared/fiber/ReactFiberUpdateQueue.js | 47 ++++++++++++++----- 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiber.js b/src/renderers/shared/fiber/ReactFiber.js index 7d03122297e74..633911db34134 100644 --- a/src/renderers/shared/fiber/ReactFiber.js +++ b/src/renderers/shared/fiber/ReactFiber.js @@ -109,7 +109,7 @@ export type Fiber = {| memoizedProps: any, // The props used to create the output. // A queue of state updates and callbacks. - updateQueue: UpdateQueue | null, + updateQueue: UpdateQueue | null, // The state used to create the output memoizedState: any, diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 6b04e36a39196..a6edb69939f34 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -22,6 +22,7 @@ var { HostPortal, CoroutineComponent, } = ReactTypeOfWork; +var {commitCallbacks} = require('ReactFiberUpdateQueue'); var {onCommitUnmount} = require('ReactFiberDevToolsHook'); var { invokeGuardedCallback, @@ -97,29 +98,6 @@ module.exports = function( } } - function commitCallbacks(updateQueue, context) { - let callbackNode = updateQueue.firstCallback; - // Reset the callback list before calling them in case something throws. - updateQueue.firstCallback = updateQueue.lastCallback = null; - - while (callbackNode !== null) { - const callback = callbackNode.callback; - // Remove this callback from the update object in case it's still part - // of the queue, so that we don't call it again. - callbackNode.callback = null; - invariant( - typeof callback === 'function', - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: %s', - callback, - ); - callback.call(context); - const nextCallback = callbackNode.nextCallback; - callbackNode.nextCallback = null; - callbackNode = nextCallback; - } - } - function commitLifeCycles(current: Fiber | null, finishedWork: Fiber): void { switch (finishedWork.tag) { case ClassComponent: { diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index f23000674e414..ae9bb6e0f7bbb 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -19,6 +19,8 @@ const {NoWork} = require('ReactFiberExpirationTime'); const {ClassComponent, HostRoot} = require('ReactTypeOfWork'); +const invariant = require('invariant'); + if (__DEV__) { var warning = require('fbjs/lib/warning'); } @@ -37,7 +39,6 @@ export type Update = { isReplace: boolean, isForced: boolean, next: Update | null, - nextCallback: Update | null, }; // Singly linked-list of updates. When an update is scheduled, it is added to @@ -61,8 +62,7 @@ export type UpdateQueue = { expirationTime: ExpirationTime, first: Update | null, last: Update | null, - firstCallback: Update | null, - lastCallback: Update | null, + callbackList: Array> | null, hasForceUpdate: boolean, // Dev only @@ -75,8 +75,7 @@ function createUpdateQueue(baseState: State): UpdateQueue { expirationTime: NoWork, first: null, last: null, - firstCallback: null, - lastCallback: null, + callbackList: null, hasForceUpdate: false, }; if (__DEV__) { @@ -202,8 +201,7 @@ function beginUpdateQueue( last: currentQueue.last, // These fields are no longer valid because they were already committed. // Reset them. - firstCallback: null, - lastCallback: null, + callbackList: null, hasForceUpdate: false, }; } @@ -279,17 +277,16 @@ function beginUpdateQueue( } if (update.callback !== null) { // Append to list of callbacks. - if (queue.lastCallback === null) { - queue.lastCallback = queue.firstCallback = update; - } else { - queue.lastCallback.nextCallback = update; - queue.lastCallback = update; + let callbackList = queue.callbackList; + if (callbackList === null) { + callbackList = queue.callbackList = []; } + callbackList.push(update); } update = update.next; } - if (queue.lastCallback !== null) { + if (queue.callbackList !== null) { workInProgress.effectTag |= CallbackEffect; } else if (queue.first === null && !queue.hasForceUpdate) { // The queue is empty. We can reset it. @@ -309,3 +306,27 @@ function beginUpdateQueue( return state; } exports.beginUpdateQueue = beginUpdateQueue; + +function commitCallbacks(queue: UpdateQueue, context: any) { + const callbackList = queue.callbackList; + if (callbackList === null) { + return; + } + // Set the list to null to make sure they don't get called more than once. + queue.callbackList = null; + for (let i = 0; i < callbackList.length; i++) { + const update = callbackList[i]; + const callback = update.callback; + // This update might be processed again. Clear the callback so it's only + // called once. + update.callback = null; + invariant( + typeof callback === 'function', + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: %s', + callback, + ); + callback.call(context); + } +} +exports.commitCallbacks = commitCallbacks; From c52e64600f552c9c41642d4a5dec5979efefeac0 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 13 Oct 2017 15:21:41 -0700 Subject: [PATCH 07/10] beginUpdateQueue -> processUpdateQueue --- src/renderers/shared/fiber/ReactFiberBeginWork.js | 6 +++--- src/renderers/shared/fiber/ReactFiberClassComponent.js | 10 +++++----- src/renderers/shared/fiber/ReactFiberUpdateQueue.js | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index ed8f6af306790..70ee077ee5b6d 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -24,7 +24,7 @@ var { reconcileChildFibersInPlace, cloneChildFibers, } = require('ReactChildFiber'); -var {beginUpdateQueue} = require('ReactFiberUpdateQueue'); +var {processUpdateQueue} = require('ReactFiberUpdateQueue'); var ReactTypeOfWork = require('ReactTypeOfWork'); var { getMaskedContext, @@ -323,7 +323,7 @@ module.exports = function( const updateQueue = workInProgress.updateQueue; if (updateQueue !== null) { const prevState = workInProgress.memoizedState; - const state = beginUpdateQueue( + const state = processUpdateQueue( current, workInProgress, updateQueue, @@ -719,7 +719,7 @@ module.exports = function( function memoizeState(workInProgress: Fiber, nextState: any) { workInProgress.memoizedState = nextState; // Don't reset the updateQueue, in case there are pending updates. Resetting - // is handled by beginUpdateQueue. + // is handled by processUpdateQueue. } function beginWork( diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index a428d27e6ac0c..df83fbe4b4559 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -26,7 +26,7 @@ var { } = require('ReactFiberContext'); var { insertUpdateIntoFiber, - beginUpdateQueue, + processUpdateQueue, } = require('ReactFiberUpdateQueue'); var {hasContextChanged} = require('ReactFiberContext'); var {isMounted} = require('ReactFiberTreeReflection'); @@ -448,7 +448,7 @@ module.exports = function( // process them now. const updateQueue = workInProgress.updateQueue; if (updateQueue !== null) { - instance.state = beginUpdateQueue( + instance.state = processUpdateQueue( current, workInProgress, updateQueue, @@ -505,7 +505,7 @@ module.exports = function( // // Process the update queue before calling shouldComponentUpdate // const updateQueue = workInProgress.updateQueue; // if (updateQueue !== null) { - // newState = beginUpdateQueue( + // newState = processUpdateQueue( // workInProgress, // updateQueue, // instance, @@ -548,7 +548,7 @@ module.exports = function( // // componentWillMount may have called setState. Process the update queue. // const newUpdateQueue = workInProgress.updateQueue; // if (newUpdateQueue !== null) { - // newState = beginUpdateQueue( + // newState = processUpdateQueue( // workInProgress, // newUpdateQueue, // instance, @@ -614,7 +614,7 @@ module.exports = function( // TODO: Previous state can be null. let newState; if (workInProgress.updateQueue !== null) { - newState = beginUpdateQueue( + newState = processUpdateQueue( current, workInProgress, workInProgress.updateQueue, diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index ae9bb6e0f7bbb..1c32b17c7e3ec 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -183,7 +183,7 @@ function getStateFromUpdate(update, instance, prevState, props) { } } -function beginUpdateQueue( +function processUpdateQueue( current: Fiber | null, workInProgress: Fiber, queue: UpdateQueue, @@ -305,7 +305,7 @@ function beginUpdateQueue( return state; } -exports.beginUpdateQueue = beginUpdateQueue; +exports.processUpdateQueue = processUpdateQueue; function commitCallbacks(queue: UpdateQueue, context: any) { const callbackList = queue.callbackList; From 794c5e5e244a61d2d37a70b6515670609676177d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 13 Oct 2017 15:31:23 -0700 Subject: [PATCH 08/10] Updates should never have an expiration of NoWork --- src/renderers/shared/fiber/ReactFiberUpdateQueue.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 1c32b17c7e3ec..40944113dcc45 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -222,10 +222,7 @@ function processUpdateQueue( let didSkip = false; while (update !== null) { const updateExpirationTime = update.expirationTime; - if ( - updateExpirationTime === NoWork || - updateExpirationTime > renderExpirationTime - ) { + if (updateExpirationTime > renderExpirationTime) { // This update does not have sufficient priority. Skip it. const remainingExpirationTime = queue.expirationTime; if ( From 729ff97f186f161ceb8b9fb445480f2ca5f24e08 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 13 Oct 2017 16:41:03 -0700 Subject: [PATCH 09/10] Rename expiration related functions --- .../shared/fiber/ReactFiberBeginWork.js | 4 +-- .../shared/fiber/ReactFiberClassComponent.js | 8 +++--- .../shared/fiber/ReactFiberExpirationTime.js | 15 +++-------- .../shared/fiber/ReactFiberReconciler.js | 10 +++---- .../shared/fiber/ReactFiberScheduler.js | 26 ++++++++++++------- 5 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 70ee077ee5b6d..42afea8aa9d4b 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -72,7 +72,7 @@ module.exports = function( hostContext: HostContext, hydrationContext: HydrationContext, scheduleWork: (fiber: Fiber, expirationTime: ExpirationTime) => void, - createUpdateExpirationForFiber: (fiber: Fiber) => ExpirationTime, + computeExpirationForFiber: (fiber: Fiber) => ExpirationTime, ) { const { shouldSetTextContent, @@ -96,7 +96,7 @@ module.exports = function( updateClassInstance, } = ReactFiberClassComponent( scheduleWork, - createUpdateExpirationForFiber, + computeExpirationForFiber, memoizeProps, memoizeState, ); diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index df83fbe4b4559..a9464b1af2dff 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -76,7 +76,7 @@ if (__DEV__) { module.exports = function( scheduleWork: (fiber: Fiber, expirationTime: ExpirationTime) => void, - createUpdateExpirationForFiber: (fiber: Fiber) => ExpirationTime, + computeExpirationForFiber: (fiber: Fiber) => ExpirationTime, memoizeProps: (workInProgress: Fiber, props: any) => void, memoizeState: (workInProgress: Fiber, state: any) => void, ) { @@ -89,7 +89,7 @@ module.exports = function( if (__DEV__) { warnOnInvalidCallback(callback, 'setState'); } - const expirationTime = createUpdateExpirationForFiber(fiber); + const expirationTime = computeExpirationForFiber(fiber); const update = { expirationTime, partialState, @@ -108,7 +108,7 @@ module.exports = function( if (__DEV__) { warnOnInvalidCallback(callback, 'replaceState'); } - const expirationTime = createUpdateExpirationForFiber(fiber); + const expirationTime = computeExpirationForFiber(fiber); const update = { expirationTime, partialState: state, @@ -127,7 +127,7 @@ module.exports = function( if (__DEV__) { warnOnInvalidCallback(callback, 'forceUpdate'); } - const expirationTime = createUpdateExpirationForFiber(fiber); + const expirationTime = computeExpirationForFiber(fiber); const update = { expirationTime, partialState: null, diff --git a/src/renderers/shared/fiber/ReactFiberExpirationTime.js b/src/renderers/shared/fiber/ReactFiberExpirationTime.js index 49c692ea241d4..8eeb26e781982 100644 --- a/src/renderers/shared/fiber/ReactFiberExpirationTime.js +++ b/src/renderers/shared/fiber/ReactFiberExpirationTime.js @@ -37,24 +37,17 @@ function ceiling(num: number, precision: number): number { return (((num / precision) | 0) + 1) * precision; } -function bucket( +function computeExpirationBucket( currentTime: ExpirationTime, expirationInMs: number, - precisionInMs: number, + bucketSizeMs: number, ): ExpirationTime { return ceiling( currentTime + expirationInMs / UNIT_SIZE, - precisionInMs / UNIT_SIZE, + bucketSizeMs / UNIT_SIZE, ); } - -// Given the current clock time, returns an expiration time. We use rounding -// to batch like updates together. -function asyncExpirationTime(currentTime: ExpirationTime) { - // Should complete within ~1000ms. 1200ms max. - return bucket(currentTime, 1000, 200); -} -exports.asyncExpirationTime = asyncExpirationTime; +exports.computeExpirationBucket = computeExpirationBucket; // Given the current clock time and an expiration time, returns the // relative expiration time. Possible values include NoWork, Sync, Task, and diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 6d0efb6c54798..e0972a0314a18 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -24,7 +24,6 @@ var {createFiberRoot} = require('ReactFiberRoot'); var ReactFiberScheduler = require('ReactFiberScheduler'); var ReactInstanceMap = require('ReactInstanceMap'); var {HostComponent} = require('ReactTypeOfWork'); -var {asyncExpirationTime} = require('ReactFiberExpirationTime'); var {insertUpdateIntoFiber} = require('ReactFiberUpdateQueue'); var emptyObject = require('fbjs/lib/emptyObject'); @@ -264,8 +263,8 @@ module.exports = function( var {getPublicInstance} = config; var { - createUpdateExpirationForFiber, - recalculateCurrentTime, + computeAsyncExpiration, + computeExpirationForFiber, scheduleWork, batchedUpdates, unbatchedUpdates, @@ -317,10 +316,9 @@ module.exports = function( element.type.prototype != null && (element.type.prototype: any).unstable_isAsyncReactComponent === true ) { - const currentTime = recalculateCurrentTime(); - expirationTime = asyncExpirationTime(currentTime); + expirationTime = computeAsyncExpiration(); } else { - expirationTime = createUpdateExpirationForFiber(current); + expirationTime = computeExpirationForFiber(current); } const update = { diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 315c04101cbda..50f16402d0662 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -59,7 +59,7 @@ var { Sync, Never, msToExpirationTime, - asyncExpirationTime, + computeExpirationBucket, relativeExpirationTime, } = require('ReactFiberExpirationTime'); @@ -166,7 +166,7 @@ module.exports = function( hostContext, hydrationContext, scheduleWork, - createUpdateExpirationForFiber, + computeExpirationForFiber, ); const {completeWork} = ReactFiberCompleteWork( config, @@ -1373,7 +1373,17 @@ module.exports = function( } } - function createUpdateExpirationForFiber(fiber: Fiber) { + function computeAsyncExpiration() { + // Given the current clock time, returns an expiration time. We use rounding + // to batch like updates together. + // Should complete within ~1000ms. 1200ms max. + const currentTime = recalculateCurrentTime(); + const expirationMs = 1000; + const bucketSizeMs = 200; + return computeExpirationBucket(currentTime, expirationMs, bucketSizeMs); + } + + function computeExpirationForFiber(fiber: Fiber) { let expirationTime; if (expirationContext !== NoWork) { // An explicit expiration context was set; @@ -1396,8 +1406,7 @@ module.exports = function( expirationTime = Sync; } else { // This is an async update - const currentTime = recalculateCurrentTime(); - expirationTime = asyncExpirationTime(currentTime); + expirationTime = computeAsyncExpiration(); } } @@ -1583,8 +1592,7 @@ module.exports = function( function deferredUpdates(fn: () => A): A { const previousExpirationContext = expirationContext; - const currentTime = recalculateCurrentTime(); - expirationContext = asyncExpirationTime(currentTime); + expirationContext = computeAsyncExpiration(); try { return fn(); } finally { @@ -1593,8 +1601,8 @@ module.exports = function( } return { - recalculateCurrentTime: recalculateCurrentTime, - createUpdateExpirationForFiber: createUpdateExpirationForFiber, + computeAsyncExpiration: computeAsyncExpiration, + computeExpirationForFiber: computeExpirationForFiber, scheduleWork: scheduleWork, batchedUpdates: batchedUpdates, unbatchedUpdates: unbatchedUpdates, From 2d49db448b63acf5822f6c9164c6fe9b1208166d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 13 Oct 2017 17:00:19 -0700 Subject: [PATCH 10/10] Fix update queue Flow types Gets rid of an unneccessary null check --- src/renderers/shared/fiber/ReactFiberBeginWork.js | 2 +- src/renderers/shared/fiber/ReactFiberUpdateQueue.js | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 42afea8aa9d4b..59499e01ca057 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -337,7 +337,7 @@ module.exports = function( resetHydrationState(); return bailoutOnAlreadyFinishedWork(current, workInProgress); } - const element = state !== null ? state.element : null; + const element = state.element; if ( (current === null || current.child === null) && enterHydrationState(workInProgress) diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index 40944113dcc45..5da8671536fef 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -19,7 +19,7 @@ const {NoWork} = require('ReactFiberExpirationTime'); const {ClassComponent, HostRoot} = require('ReactTypeOfWork'); -const invariant = require('invariant'); +const invariant = require('fbjs/lib/invariant'); if (__DEV__) { var warning = require('fbjs/lib/warning'); @@ -57,7 +57,7 @@ export type UpdateQueue = { // unprocessed updates that came before it. In that case, we need to keep // track of the base state, which represents the base state of the first // unprocessed update, which is the same as the first update in the list. - baseState: State | null, + baseState: State, // For the same reason, we keep track of the remaining expiration time. expirationTime: ExpirationTime, first: Update | null, @@ -87,7 +87,7 @@ function createUpdateQueue(baseState: State): UpdateQueue { function insertUpdateIntoQueue( queue: UpdateQueue, update: Update, -) { +): void { // Append the update to the end of the list. if (queue.last === null) { // Queue is empty @@ -105,7 +105,10 @@ function insertUpdateIntoQueue( } exports.insertUpdateIntoQueue = insertUpdateIntoQueue; -function insertUpdateIntoFiber(fiber: Fiber, update: Update) { +function insertUpdateIntoFiber( + fiber: Fiber, + update: Update, +): void { // We'll have at least one and at most two distinct update queues. const alternateFiber = fiber.alternate; let queue1 = fiber.updateQueue; @@ -190,7 +193,7 @@ function processUpdateQueue( instance: any, props: any, renderExpirationTime: ExpirationTime, -): any { +): State { if (current !== null && current.updateQueue === queue) { // We need to create a work-in-progress queue, by cloning the current queue. const currentQueue = queue;