Skip to content

Commit

Permalink
Committing a update on one root should not flush work on another root
Browse files Browse the repository at this point in the history
`forceExpire` is tracked per root rather than per scheduler.
  • Loading branch information
acdlite committed Sep 29, 2017
1 parent 40806dc commit 6cee370
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/renderers/shared/fiber/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
return;
}
processUpdateQueue(blockers, null, null, null, expirationTime);
expireWork(expirationTime);
expireWork(root, expirationTime);
};
WorkNode.prototype.then = function(callback) {
const root = this._reactRootContainer;
Expand Down
4 changes: 4 additions & 0 deletions src/renderers/shared/fiber/ReactFiberRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ export type FiberRoot = {
// A queue of callbacks that fire once their corresponding expiration time
// has completed. Only fired once.
completionCallbacks: UpdateQueue<null> | 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
Expand Down Expand Up @@ -64,6 +67,7 @@ exports.createFiberRoot = function(containerInfo: any): FiberRoot {
completedAt: Done,
blockers: null,
completionCallbacks: null,
forceExpire: null,
nextScheduledRoot: null,
context: null,
pendingContext: null,
Expand Down
24 changes: 13 additions & 11 deletions src/renderers/shared/fiber/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
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;
Expand Down Expand Up @@ -305,7 +302,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
(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.
Expand Down Expand Up @@ -1723,25 +1720,30 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
}

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();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(<span prop="A" />);
Expand All @@ -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 <span prop={props.label} />;
}

// Prerender work on two separate roots
const workA = rootA.prerender(<Foo label="A" />);
rootB.prerender(<Foo label="B" />);

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']);
});
});

0 comments on commit 6cee370

Please sign in to comment.