From 9dba218d933d66fba9a8ba23b90dbb852514da8b Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 4 Feb 2020 11:35:21 -0800 Subject: [PATCH] [Mock Scheduler] Mimic browser's advanceTime (#17967) 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. --- .../__tests__/ChangeEventPlugin-test.internal.js | 1 + .../ReactIncrementalUpdates-test.internal.js | 4 ++-- .../ReactSuspenseWithNoopRenderer-test.internal.js | 4 ++-- packages/scheduler/src/__tests__/Scheduler-test.js | 10 +++++----- .../scheduler/src/forks/SchedulerHostConfig.mock.js | 11 ++++------- 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js index c39febb976e5c..8357dbc633c64 100644 --- a/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js +++ b/packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js @@ -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'); }); }, diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js index bcb9f7189c05a..231dcde677eb1 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js @@ -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']); }); @@ -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', diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index 82b802bc087a6..71dd2c8df71b0 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -848,7 +848,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { , ); Scheduler.unstable_advanceTime(10000); - expect(Scheduler).toHaveYielded([ + expect(Scheduler).toFlushExpired([ 'Suspend! [A]', 'Suspend! [B]', 'Loading...', @@ -987,7 +987,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { Scheduler.unstable_advanceTime(10000); jest.advanceTimersByTime(10000); - expect(Scheduler).toHaveYielded([ + expect(Scheduler).toFlushExpired([ 'Suspend! [goodbye]', 'Loading...', 'Commit: goodbye', diff --git a/packages/scheduler/src/__tests__/Scheduler-test.js b/packages/scheduler/src/__tests__/Scheduler-test.js index 881d6d9ffe154..51de6f7b55864 100644 --- a/packages/scheduler/src/__tests__/Scheduler-test.js +++ b/packages/scheduler/src/__tests__/Scheduler-test.js @@ -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([ @@ -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', () => { @@ -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', () => { @@ -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', () => { diff --git a/packages/scheduler/src/forks/SchedulerHostConfig.mock.js b/packages/scheduler/src/forks/SchedulerHostConfig.mock.js index 3657092335941..cef9f1b2e1024 100644 --- a/packages/scheduler/src/forks/SchedulerHostConfig.mock.js +++ b/packages/scheduler/src/forks/SchedulerHostConfig.mock.js @@ -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; } }