Skip to content

Commit

Permalink
[Mock Scheduler] Mimic browser's advanceTime (#17967)
Browse files Browse the repository at this point in the history
The mock Scheduler that we use in our tests has its own fake timer
implementation. The `unstable_advanceTime` method advances the timeline.

Currently, a call to `unstable_advanceTime` will also flush any pending
expired work. But that's not how it works in the browser: when a timer
fires, the corresponding task is added to the Scheduler queue. However,
we will still wait until the next message event before flushing it.

This commit changes `unstable_advanceTime` to more closely resemble the
browser behavior, by removing the automatic flushing of expired work.

```js
// Before this commit
Scheduler.unstable_advanceTime(ms);

// Equivalent behavior after this commit
Scheduler.unstable_advanceTime(ms);
Scheduler.unstable_flushExpired();
```

The general principle is to prefer separate APIs for scheduling tasks
and flushing them.

This change does not affect any public APIs. `unstable_advanceTime` is
only used by our own test suite. It is not used by `act`.

However, we may need to update tests in www, like Relay's.
  • Loading branch information
acdlite authored Feb 4, 2020
1 parent cddde45 commit 9dba218
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,7 @@ describe('ChangeEventPlugin', () => {

// 3s should be enough to expire the updates
Scheduler.unstable_advanceTime(3000);
expect(Scheduler).toFlushExpired([]);
expect(container.textContent).toEqual('hovered');
});
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ describe('ReactIncrementalUpdates', () => {

// All the updates should render and commit in a single batch.
Scheduler.unstable_advanceTime(10000);
expect(Scheduler).toHaveYielded(['Render: goodbye']);
expect(Scheduler).toFlushExpired(['Render: goodbye']);
// Passive effect
expect(Scheduler).toFlushAndYield(['Commit: goodbye']);
});
Expand Down Expand Up @@ -645,7 +645,7 @@ describe('ReactIncrementalUpdates', () => {

// All the updates should render and commit in a single batch.
Scheduler.unstable_advanceTime(10000);
expect(Scheduler).toHaveYielded([
expect(Scheduler).toFlushExpired([
'Render: goodbye',
'Commit: goodbye',
'Render: goodbye',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
</Suspense>,
);
Scheduler.unstable_advanceTime(10000);
expect(Scheduler).toHaveYielded([
expect(Scheduler).toFlushExpired([
'Suspend! [A]',
'Suspend! [B]',
'Loading...',
Expand Down Expand Up @@ -987,7 +987,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
Scheduler.unstable_advanceTime(10000);
jest.advanceTimersByTime(10000);

expect(Scheduler).toHaveYielded([
expect(Scheduler).toFlushExpired([
'Suspend! [goodbye]',
'Loading...',
'Commit: goodbye',
Expand Down
10 changes: 5 additions & 5 deletions packages/scheduler/src/__tests__/Scheduler-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,14 @@ describe('Scheduler', () => {

// Advance by just a bit more to expire the user blocking callbacks
Scheduler.unstable_advanceTime(1);
expect(Scheduler).toHaveYielded([
expect(Scheduler).toFlushExpired([
'B (did timeout: true)',
'C (did timeout: true)',
]);

// Expire A
Scheduler.unstable_advanceTime(4600);
expect(Scheduler).toHaveYielded(['A (did timeout: true)']);
expect(Scheduler).toFlushExpired(['A (did timeout: true)']);

// Flush the rest without expiring
expect(Scheduler).toFlushAndYield([
Expand All @@ -140,7 +140,7 @@ describe('Scheduler', () => {
expect(Scheduler).toHaveYielded([]);

Scheduler.unstable_advanceTime(1);
expect(Scheduler).toHaveYielded(['A']);
expect(Scheduler).toFlushExpired(['A']);
});

it('continues working on same task after yielding', () => {
Expand Down Expand Up @@ -217,7 +217,7 @@ describe('Scheduler', () => {

// Advance time by just a bit more. This should expire all the remaining work.
Scheduler.unstable_advanceTime(1);
expect(Scheduler).toHaveYielded(['C', 'D']);
expect(Scheduler).toFlushExpired(['C', 'D']);
});

it('continuations are interrupted by higher priority work', () => {
Expand Down Expand Up @@ -705,7 +705,7 @@ describe('Scheduler', () => {

// Now it expires
Scheduler.unstable_advanceTime(1);
expect(Scheduler).toHaveYielded(['A']);
expect(Scheduler).toFlushExpired(['A']);
});

it('cancels a delayed task', () => {
Expand Down
11 changes: 4 additions & 7 deletions packages/scheduler/src/forks/SchedulerHostConfig.mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,10 @@ export function unstable_yieldValue(value: mixed): void {

export function unstable_advanceTime(ms: number) {
currentTime += ms;
if (!isFlushing) {
if (scheduledTimeout !== null && timeoutTime <= currentTime) {
scheduledTimeout(currentTime);
timeoutTime = -1;
scheduledTimeout = null;
}
unstable_flushExpired();
if (scheduledTimeout !== null && timeoutTime <= currentTime) {
scheduledTimeout(currentTime);
timeoutTime = -1;
scheduledTimeout = null;
}
}

Expand Down

0 comments on commit 9dba218

Please sign in to comment.