diff --git a/package.json b/package.json index c019a7d717fc5..0826d8516c09a 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..133d58413b751 100644 --- a/scripts/jest/test-framework-setup.js +++ b/scripts/jest/test-framework-setup.js @@ -28,7 +28,7 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { env.afterEach(() => { if (console[methodName] !== newMethod && !isSpy(console[methodName])) { throw new Error( - 'Test did not tear down console.' + methodName + ' mock properly.' + 'Test did not tear down console.' + methodName + ' mock properly.', ); } if (console[methodName].__callCount !== 0) { @@ -40,7 +40,7 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { "spyOn(console, '" + methodName + "') and test that the " + - 'warning occurs.' + 'warning occurs.', ); } }); @@ -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 0b96592359c3b..cabd6d3d177bf 100644 --- a/src/renderers/dom/fiber/__tests__/ReactDOMFiberAsync-test.js +++ b/src/renderers/dom/fiber/__tests__/ReactDOMFiberAsync-test.js @@ -286,8 +286,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 40f20e54174b2..be5951acc8a43 100644 --- a/src/renderers/noop/ReactNoopEntry.js +++ b/src/renderers/noop/ReactNoopEntry.js @@ -298,9 +298,18 @@ var ReactNoop = { const root = NoopRenderer.createContainer(container); roots.set(rootID, root); return { + render(children: any) { + const work = NoopRenderer.updateRoot(children, root, null); + work.then(() => work.commit()); + }, prerender(children: any) { return NoopRenderer.updateRoot(children, root, null); }, + unmount() { + roots.delete(rootID); + const work = NoopRenderer.updateRoot(null, root, null); + work.then(() => work.commit()); + }, getChildren() { return ReactNoop.getChildren(rootID); }, diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 638c845c7e3b1..fcec45b97f86c 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -352,7 +352,6 @@ module.exports = function( workInProgress, updateQueue, null, - prevState, null, renderExpirationTime, ); @@ -362,7 +361,7 @@ module.exports = function( resetHydrationState(); return bailoutOnAlreadyFinishedWork(current, workInProgress); } - const element = state.element; + const element = state !== null ? state.element : null; if ( root.hydrate && (current === null || current.child === null) && diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 977ed605a1266..c2851c11ed1e9 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -113,7 +113,7 @@ module.exports = function( callback, isReplace: false, isForced: false, - isTopLevelUnmount: false, + nextCallback: null, next: null, }; insertUpdateIntoFiber(fiber, update, currentTime); @@ -138,7 +138,7 @@ module.exports = function( callback, isReplace: true, isForced: false, - isTopLevelUnmount: false, + nextCallback: null, next: null, }; insertUpdateIntoFiber(fiber, update, currentTime); @@ -163,7 +163,7 @@ module.exports = function( callback, isReplace: false, isForced: true, - isTopLevelUnmount: false, + nextCallback: null, next: null, }; insertUpdateIntoFiber(fiber, update, currentTime); @@ -458,7 +458,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); @@ -482,7 +482,6 @@ module.exports = function( workInProgress, updateQueue, instance, - state, props, renderExpirationTime, ); @@ -649,7 +648,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 99c03b2201498..5ca410923514d 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -31,12 +31,7 @@ var { clearCaughtError, } = require('ReactErrorUtils'); -var { - Placement, - Update, - Callback, - ContentReset, -} = require('ReactTypeOfSideEffect'); +var {Placement, Update, ContentReset} = require('ReactTypeOfSideEffect'); var invariant = require('fbjs/lib/invariant'); @@ -489,9 +484,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 ' + @@ -499,6 +501,9 @@ module.exports = function( callback, ); callback.call(context); + const nextCallback = callbackNode.nextCallback; + callbackNode.nextCallback = null; + callbackNode = nextCallback; } } @@ -531,31 +536,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 a1a4eb172c212..c08e130756814 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -278,7 +278,6 @@ module.exports = function( callback, ); } - const isTopLevelUnmount = nextState.element === null; const update = { priorityLevel, expirationTime, @@ -286,35 +285,15 @@ module.exports = function( callback, isReplace: false, isForced: false, - isTopLevelUnmount, + nextCallback: null, next: null, }; - const update2 = insertUpdateIntoFiber(current, update, currentTime); - - 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, currentTime); if (isPrerender) { // Block the root from committing at this expiration time. if (root.blockers === null) { - root.blockers = createUpdateQueue(); + root.blockers = createUpdateQueue(null); } const block = { priorityLevel: null, @@ -323,7 +302,7 @@ module.exports = function( callback: null, isReplace: false, isForced: false, - isTopLevelUnmount: false, + nextCallback: null, next: null, }; insertUpdateIntoQueue(root.blockers, block, currentTime); @@ -344,7 +323,7 @@ module.exports = function( if (blockers === null) { return; } - processUpdateQueue(blockers, null, null, null, expirationTime); + processUpdateQueue(blockers, null, null, expirationTime); expireWork(root, expirationTime); }; WorkNode.prototype.then = function(callback) { @@ -395,7 +374,7 @@ module.exports = function( let completionCallbacks = container.completionCallbacks; if (completionCallbacks === null) { - completionCallbacks = createUpdateQueue(); + completionCallbacks = createUpdateQueue(null); } return new WorkNode(container, expirationTime); diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 69ac7e39d2f08..1a821d6ecbf34 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -416,21 +416,23 @@ module.exports = function( // the end of the current batch. const completionCallbacks = root.completionCallbacks; if (completionCallbacks !== null) { - processUpdateQueue(completionCallbacks, null, null, null, completedAt); - const callbackList = completionCallbacks.callbackList; - if (callbackList !== null) { - // Add new callbacks to list of completion callbacks + processUpdateQueue(completionCallbacks, null, null, completedAt); + // Add new callbacks to list of completion callbacks + let callbackNode = completionCallbacks.firstCallback; + completionCallbacks.firstCallback = completionCallbacks.lastCallback = null; + while (callbackNode !== null) { + const callback: () => mixed = (callbackNode.callback: any); + // 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; if (rootCompletionCallbackList === null) { - rootCompletionCallbackList = callbackList; + rootCompletionCallbackList = [callback]; } else { - for (let i = 0; i < callbackList.length; i++) { - rootCompletionCallbackList.push(callbackList[i]); - } - } - completionCallbacks.callbackList = null; - if (completionCallbacks.first === null) { - root.completionCallbacks = null; + rootCompletionCallbackList.push(callback); } + const nextCallback = callbackNode.nextCallback; + callbackNode.nextCallback = null; + callbackNode = nextCallback; } } } @@ -1642,12 +1644,12 @@ module.exports = function( callback, isReplace: false, isForced: false, - isTopLevelUnmount: false, + nextCallback: null, next: null, }; const currentTime = recalculateCurrentTime(); if (root.completionCallbacks === null) { - root.completionCallbacks = createUpdateQueue(); + root.completionCallbacks = createUpdateQueue(null); } insertUpdateIntoQueue(root.completionCallbacks, update, currentTime); if (expirationTime === root.completedAt) { diff --git a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js index c022f11e14921..432afae883434 100644 --- a/src/renderers/shared/fiber/ReactFiberUpdateQueue.js +++ b/src/renderers/shared/fiber/ReactFiberUpdateQueue.js @@ -39,8 +39,8 @@ export type Update = { callback: Callback | null, isReplace: boolean, isForced: boolean, - isTopLevelUnmount: boolean, next: Update | null, + nextCallback: Update | null, }; // Singly linked-list of updates. When an update is scheduled, it is added to @@ -55,24 +55,32 @@ export type Update = { // // When the tree is committed, the work-in-progress becomes the current. 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 { +function createUpdateQueue(baseState: State): UpdateQueue { const queue: UpdateQueue = { + baseState, + expirationTime: Done, first: null, last: null, + firstCallback: null, + lastCallback: null, hasForceUpdate: false, - callbackList: null, }; if (__DEV__) { queue.isProcessing = false; @@ -81,147 +89,90 @@ function createUpdateQueue(): UpdateQueue { } exports.createUpdateQueue = createUpdateQueue; -function cloneUpdate(update: Update): Update { - return { - priorityLevel: update.priorityLevel, - expirationTime: update.expirationTime, - partialState: update.partialState, - callback: update.callback, - isReplace: update.isReplace, - isForced: update.isForced, - isTopLevelUnmount: update.isTopLevelUnmount, - next: null, - }; -} - const COALESCENCE_THRESHOLD: ExpirationTime = 10; -function insertUpdateIntoPosition( +function insertUpdateIntoQueue( queue: UpdateQueue, update: Update, - insertAfter: Update | null, - insertBefore: Update | null, currentTime: ExpirationTime, ) { - if (insertAfter !== null) { - insertAfter.next = update; - // If we receive multiple updates to the same fiber at the same priority - // level, we coalesce them by assigning the same expiration time, so that - // they all flush at the same time. Because this causes an interruption, it - // could lead to starvation, so we stop coalescing once the time until the - // expiration time reaches a certain threshold. + if (queue.last === null) { + // Queue is empty + queue.first = queue.last = update; if ( - // Only coalesce if a priority level is specified - update.priorityLevel !== null && - insertAfter !== null && - insertAfter.priorityLevel === update.priorityLevel + queue.expirationTime === Done || + queue.expirationTime > update.expirationTime ) { - const coalescedTime = insertAfter.expirationTime; + queue.expirationTime = update.expirationTime; + } + return; + } + + // If the priority is not null, see if there are other pending updates with + // the same priority. If so, we may need to coalesce them into the same + // expiration bucket. + const priorityLevel = update.priorityLevel; + if (priorityLevel !== null) { + let coalescentUpdate = null; + if (queue.last.priorityLevel === priorityLevel) { + // Fast path where the last update has the same priority. + coalescentUpdate = queue.last; + } else { + // Scan the list for the last update with the same priority. + let node = queue.first; + while (node !== null) { + if (node.priorityLevel === priorityLevel) { + coalescentUpdate = node; + } + node = node.next; + } + } + if (coalescentUpdate !== null) { + const coalescedTime = coalescentUpdate.expirationTime; + // If the remaining time is greater than the threshold, add the new + // update to the same expiration bucket. if (coalescedTime - currentTime > COALESCENCE_THRESHOLD) { update.expirationTime = coalescedTime; } } - } else { - // This is the first item in the queue. - update.next = queue.first; - queue.first = update; } - if (insertBefore !== null) { - update.next = insertBefore; - } else { - // This is the last item in the queue. - queue.last = update; + // Append the update to the end of the list. + queue.last.next = update; + queue.last = update; + if ( + queue.expirationTime === Done || + queue.expirationTime > update.expirationTime + ) { + queue.expirationTime = update.expirationTime; } } +exports.insertUpdateIntoQueue = insertUpdateIntoQueue; -// 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: UpdateQueue, +function insertUpdateIntoFiber( + fiber: Fiber, update: 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; - } - } - return insertAfter; -} - -function ensureUpdateQueues(fiber: Fiber) { + currentTime: ExpirationTime, +) { + // 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, - currentTime: ExpirationTime, -): 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__) { @@ -236,96 +187,34 @@ function insertUpdateIntoFiber( } } - // 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, - currentTime, - ); - return null; + insertUpdateIntoQueue(queue1, update, currentTime); + 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, - currentTime, - ); - - // 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, - currentTime, - ); - return update2; + // If either queue is empty, we need to add to both queues. + if (queue1.last === null || queue2.last === null) { + insertUpdateIntoQueue(queue1, update, currentTime); + insertUpdateIntoQueue(queue2, update, currentTime); + return; } -} -exports.insertUpdateIntoFiber = insertUpdateIntoFiber; -function insertUpdateIntoQueue( - queue: UpdateQueue, - update: Update, - currentTime: ExpirationTime, -) { - const insertAfter = findInsertionPosition(queue, update); - const insertBefore = insertAfter !== null ? insertAfter.next : null; - insertUpdateIntoPosition( - queue, - update, - insertAfter, - insertBefore, - currentTime, - ); + // 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, currentTime); + // But we still need to update the `last` pointer of queue2. + queue2.last = update; } -exports.insertUpdateIntoQueue = insertUpdateIntoQueue; +exports.insertUpdateIntoFiber = insertUpdateIntoFiber; function getUpdateExpirationTime(fiber: Fiber): ExpirationTime { - const updateQueue = fiber.updateQueue; - if (updateQueue === null) { + if (fiber.tag !== ClassComponent && fiber.tag !== HostRoot) { return Done; } - if (fiber.tag !== ClassComponent && fiber.tag !== HostRoot) { + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { return Done; } return getUpdateQueueExpirationTime(updateQueue); @@ -335,7 +224,7 @@ exports.getUpdateExpirationTime = getUpdateExpirationTime; function getUpdateQueueExpirationTime( updateQueue: UpdateQueue, ): ExpirationTime { - return updateQueue.first !== null ? updateQueue.first.expirationTime : Done; + return updateQueue.expirationTime; } exports.getUpdateQueueExpirationTime = getUpdateQueueExpirationTime; @@ -353,34 +242,59 @@ function getStateFromUpdate(update, instance, prevState, props) { function processUpdateQueue( queue: UpdateQueue, instance: mixed, - prevState: State, props: mixed, renderExpirationTime: ExpirationTime, -): State { +): State | null { if (__DEV__) { // Set this flag so we can warn if setState is called inside the update // function of another setState. 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 = Done; - // 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 === Done || + updateExpirationTime > renderExpirationTime + ) { + // This update does not have sufficient priority. Skip it. + const remainingExpirationTime = queue.expirationTime; + if ( + remainingExpirationTime === Done || + 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); @@ -398,22 +312,23 @@ function processUpdateQueue( } } 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; } - - queue.callbackList = callbackList; - queue.hasForceUpdate = hasForceUpdate; + if (!didSkip) { + didSkip = true; + queue.baseState = state; + } if (__DEV__) { // No longer processing. @@ -429,19 +344,21 @@ function beginUpdateQueue( workInProgress: Fiber, queue: UpdateQueue, instance: any, - prevState: any, props: any, renderExpirationTime: ExpirationTime, -): State { +): State | null { if (current !== null && current.updateQueue === queue) { // 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, }; } @@ -449,15 +366,13 @@ function beginUpdateQueue( const state = processUpdateQueue( queue, instance, - prevState, props, renderExpirationTime, ); const updatedQueue = workInProgress.updateQueue; if (updatedQueue !== null) { - const callbackList = updatedQueue.callbackList; - if (callbackList !== null) { + if (updatedQueue.lastCallback !== null) { workInProgress.effectTag |= CallbackEffect; } else if (updatedQueue.first === null && !updatedQueue.hasForceUpdate) { // The queue is empty. We can reset it. diff --git a/src/renderers/shared/fiber/__tests__/ReactDeterministicUpdates-test.js b/src/renderers/shared/fiber/__tests__/ReactDeterministicUpdates-test.js new file mode 100644 index 0000000000000..294da25f75c24 --- /dev/null +++ b/src/renderers/shared/fiber/__tests__/ReactDeterministicUpdates-test.js @@ -0,0 +1,110 @@ +/** + * 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('ReactDeterministicUpdates', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactNoop = require('ReactNoopEntry'); + }); + + const HIGH_PRI_UPDATE = 'HIGH_PRI_UPDATE'; + const LOW_PRI_UPDATE = 'LOW_PRI_UPDATE'; + const EXPIRE = 'EXPIRE'; + + const genAction = gen.oneOf([ + {type: HIGH_PRI_UPDATE}, + {type: LOW_PRI_UPDATE}, + gen.object({type: EXPIRE, payload: gen.intWithin(0, 50)}), + ]); + + check.it( + 'terminal value is the same regardless of priority', + {times: 20}, + gen.array(genAction, {size: 5}), + (actions, id) => { + class Log extends React.Component { + state = {log: []}; + render() { + return null; + } + } + + let instance; + const root = ReactNoop.createRoot(); + root.render( (instance = inst)} />); + ReactNoop.flush(); + + // Schedule updates at different priorities. Assert that they are + // processed in insertion order. + function updateLog(value) { + // Append value to the log. The log represents the order in which + // updates were processed. + return state => ({log: [...state.log, value]}); + } + + function checkLog(log) { + let previous; + for (let i = 0; i < log.length; i++) { + const current = log[i]; + if (typeof current !== 'number') { + throw new Error('Expected a number.'); + } + if (previous !== undefined) { + if (current < previous) { + throw new Error('Updates were flushed out of order'); + } + } + previous = current; + } + } + + try { + var updateCounter = 0; + var expectedTerminalLog = []; + for (var i = 0; i < actions.length; i++) { + var action = actions[i]; + ReactNoop.flushSync(() => { + switch (action.type) { + case HIGH_PRI_UPDATE: + instance.setState(updateLog(++updateCounter)); + expectedTerminalLog.push(updateCounter); + break; + case LOW_PRI_UPDATE: + ReactNoop.deferredUpdates(() => { + instance.setState(updateLog(++updateCounter)); + }); + expectedTerminalLog.push(updateCounter); + break; + case EXPIRE: + ReactNoop.expire(action.payload); + break; + default: + throw new Error('Switch statement should be exhaustive.'); + } + }); + checkLog(instance.state.log); + } + ReactNoop.flush(); + // The final state is deterministic + expect(instance.state.log).toEqual(expectedTerminalLog); + } finally { + root.unmount(); + ReactNoop.flush(); + } + }, + ); +}); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js index 3ed31d9456e7c..4adae7d3eafb7 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js @@ -59,11 +59,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 15c20bb8a0c43..cb4261a37af40 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), toggle(0), flush(2), step(2), toggle(0)); simulate(step(1), flush(3), toggle(0), step(0)); simulate(step(1), flush(3), toggle(18), step(0)); + 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 7879162ad54de..24ca48c4eb85a 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js @@ -156,19 +156,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')]); }); @@ -204,23 +205,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 fdacfca48ba9c..44a014b73fd61 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2602,6 +2602,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" @@ -4303,6 +4309,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"