From 2e7eac6c3bcf14f53cbfe5efebc20dbc6c67bc19 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 11 Sep 2017 15:58:13 -0700 Subject: [PATCH] Committing a update on one root should not flush work on another root `forceExpire` is tracked per root rather than per scheduler. --- .../shared/fiber/ReactFiberReconciler.js | 2 +- src/renderers/shared/fiber/ReactFiberRoot.js | 4 +++ .../shared/fiber/ReactFiberScheduler.js | 24 ++++++++------- .../__tests__/ReactIncrementalRoot-test.js | 30 ++++++++++++++++++- 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index ab9acc773c186f..384d87834403ae 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -344,7 +344,7 @@ module.exports = function( return; } processUpdateQueue(blockers, null, null, null, expirationTime); - expireWork(expirationTime); + expireWork(root, expirationTime); }; WorkNode.prototype.then = function(callback) { const root = this._reactRootContainer; diff --git a/src/renderers/shared/fiber/ReactFiberRoot.js b/src/renderers/shared/fiber/ReactFiberRoot.js index 613e63b314a343..b22476025bb3ec 100644 --- a/src/renderers/shared/fiber/ReactFiberRoot.js +++ b/src/renderers/shared/fiber/ReactFiberRoot.js @@ -36,6 +36,9 @@ export type FiberRoot = { // A queue of callbacks that fire once their corresponding expiration time // has completed. Only fired once. completionCallbacks: UpdateQueue | null, + // When set, indicates that all work in this tree with this time or earlier + // should be flushed by the end of the batch, as if it has task priority. + forceExpire: null | ExpirationTime, // The work schedule is a linked list. nextScheduledRoot: FiberRoot | null, // Top context object, used by renderSubtreeIntoContainer @@ -66,6 +69,7 @@ exports.createFiberRoot = function(containerInfo: any): FiberRoot { completedAt: Done, blockers: null, completionCallbacks: null, + forceExpire: null, nextScheduledRoot: null, context: null, pendingContext: null, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 1122e32ec99d2f..0e941e1726296d 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -225,9 +225,6 @@ module.exports = function( let nextUnitOfWork: Fiber | null = null; // The time at which we're currently rendering work. let nextRenderExpirationTime: ExpirationTime = Done; - // If not null, all work up to and including this time should be - // flushed before the end of the current batch. - let forceExpire: ExpirationTime | null = null; // The next fiber with an effect that we're currently committing. let nextEffect: Fiber | null = null; @@ -307,7 +304,7 @@ module.exports = function( (earliestExpirationTime === Done || earliestExpirationTime > rootExpirationTime) ) { - earliestExpirationTime = root.current.expirationTime; + earliestExpirationTime = rootExpirationTime; earliestExpirationRoot = root; } // We didn't find anything to do in this root, so let's try the next one. @@ -1717,25 +1714,30 @@ module.exports = function( } function recalculateCurrentTime(): ExpirationTime { - if (forceExpire !== null) { - return forceExpire; + if (nextRenderedTree !== null) { + // Check if the current root is being force expired. + const forceExpire = nextRenderedTree.forceExpire; + if (forceExpire !== null) { + // Override the current time with the `forceExpire` time. This has the + // effect of expiring all work up to and including that time. + mostRecentCurrentTime = forceExpire; + return forceExpire; + } } mostRecentCurrentTime = msToExpirationTime(now()); return mostRecentCurrentTime; } - function expireWork(expirationTime: ExpirationTime): void { + function expireWork(root: FiberRoot, expirationTime: ExpirationTime): void { invariant( !isPerformingWork, 'Cannot commit while already performing work.', ); - // Override the current time with the given time. This has the effect of - // expiring all work up to and including that time. - forceExpire = mostRecentCurrentTime = expirationTime; + root.forceExpire = expirationTime; try { performWork(TaskPriority, null); } finally { - forceExpire = null; + root.forceExpire = null; recalculateCurrentTime(); } } diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalRoot-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalRoot-test.js index 9bfd77f099fcfc..fd006b69d62604 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalRoot-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalRoot-test.js @@ -135,7 +135,7 @@ describe('ReactIncrementalRoot', () => { expect(root.getChildren()).toEqual([span('A')]); }); - it('flushes ealier work if later work is committed', () => { + it('flushes earlier work if later work is committed', () => { let ops = []; const root = ReactNoop.createRoot(); const work1 = root.prerender(); @@ -153,4 +153,32 @@ describe('ReactIncrementalRoot', () => { expect(root.getChildren()).toEqual([span('B')]); expect(ops).toEqual(['complete 1', 'complete 2']); }); + + it('committing work on one tree does not commit or expire work in a separate tree', () => { + let ops = []; + + const rootA = ReactNoop.createRoot('A'); + const rootB = ReactNoop.createRoot('B'); + + function Foo(props) { + ops.push(props.label); + return ; + } + + // Prerender work on two separate roots + const workA = rootA.prerender(); + rootB.prerender(); + + expect(rootA.getChildren()).toEqual([]); + expect(rootB.getChildren()).toEqual([]); + expect(ops).toEqual([]); + + // Commit root A. This forces the remaining work on root A to expire, but + // should not expire work on root B. + workA.commit(); + + expect(rootA.getChildren()).toEqual([span('A')]); + expect(rootB.getChildren()).toEqual([]); + expect(ops).toEqual(['A']); + }); });