@samitouri / QOS-React-2 / commits / 9dba218d93

[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.

Andrew Clark committed Feb 4, 2020 at 11:35 UTC 9dba218d933d66fba9a8ba23b90dbb852514da8b
5 files changed +14 -16
packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js
+1
@@ -779,6 +779,7 @@ describe('ChangeEventPlugin', () => {
779
780 // 3s should be enough to expire the updates
781 Scheduler.unstable_advanceTime(3000);
782 + expect(Scheduler).toFlushExpired([]);
783 expect(container.textContent).toEqual('hovered');
784 });
785 },
packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js
+2 -2
@@ -540,7 +540,7 @@ describe('ReactIncrementalUpdates', () => {
540
541 // All the updates should render and commit in a single batch.
542 Scheduler.unstable_advanceTime(10000);
543 - expect(Scheduler).toHaveYielded(['Render: goodbye']);
543 + expect(Scheduler).toFlushExpired(['Render: goodbye']);
544 // Passive effect
545 expect(Scheduler).toFlushAndYield(['Commit: goodbye']);
546 });
@@ -645,7 +645,7 @@ describe('ReactIncrementalUpdates', () => {
645
646 // All the updates should render and commit in a single batch.
647 Scheduler.unstable_advanceTime(10000);
648 - expect(Scheduler).toHaveYielded([
648 + expect(Scheduler).toFlushExpired([
649 'Render: goodbye',
650 'Commit: goodbye',
651 'Render: goodbye',
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
+2 -2
@@ -848,7 +848,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
848 </Suspense>,
849 );
850 Scheduler.unstable_advanceTime(10000);
851 - expect(Scheduler).toHaveYielded([
851 + expect(Scheduler).toFlushExpired([
852 'Suspend! [A]',
853 'Suspend! [B]',
854 'Loading...',
@@ -987,7 +987,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
987 Scheduler.unstable_advanceTime(10000);
988 jest.advanceTimersByTime(10000);
989
990 - expect(Scheduler).toHaveYielded([
990 + expect(Scheduler).toFlushExpired([
991 'Suspend! [goodbye]',
992 'Loading...',
993 'Commit: goodbye',
packages/scheduler/src/__tests__/Scheduler-test.js
+5 -5
@@ -117,14 +117,14 @@ describe('Scheduler', () => {
117
118 // Advance by just a bit more to expire the user blocking callbacks
119 Scheduler.unstable_advanceTime(1);
120 - expect(Scheduler).toHaveYielded([
120 + expect(Scheduler).toFlushExpired([
121 'B (did timeout: true)',
122 'C (did timeout: true)',
123 ]);
124
125 // Expire A
126 Scheduler.unstable_advanceTime(4600);
127 - expect(Scheduler).toHaveYielded(['A (did timeout: true)']);
127 + expect(Scheduler).toFlushExpired(['A (did timeout: true)']);
128
129 // Flush the rest without expiring
130 expect(Scheduler).toFlushAndYield([
@@ -140,7 +140,7 @@ describe('Scheduler', () => {
140 expect(Scheduler).toHaveYielded([]);
141
142 Scheduler.unstable_advanceTime(1);
143 - expect(Scheduler).toHaveYielded(['A']);
143 + expect(Scheduler).toFlushExpired(['A']);
144 });
145
146 it('continues working on same task after yielding', () => {
@@ -217,7 +217,7 @@ describe('Scheduler', () => {
217
218 // Advance time by just a bit more. This should expire all the remaining work.
219 Scheduler.unstable_advanceTime(1);
220 - expect(Scheduler).toHaveYielded(['C', 'D']);
220 + expect(Scheduler).toFlushExpired(['C', 'D']);
221 });
222
223 it('continuations are interrupted by higher priority work', () => {
@@ -705,7 +705,7 @@ describe('Scheduler', () => {
705
706 // Now it expires
707 Scheduler.unstable_advanceTime(1);
708 - expect(Scheduler).toHaveYielded(['A']);
708 + expect(Scheduler).toFlushExpired(['A']);
709 });
710
711 it('cancels a delayed task', () => {
packages/scheduler/src/forks/SchedulerHostConfig.mock.js
+4 -7
@@ -201,13 +201,10 @@ export function unstable_yieldValue(value: mixed): void {
201
202 export function unstable_advanceTime(ms: number) {
203 currentTime += ms;
204 - if (!isFlushing) {
205 - if (scheduledTimeout !== null && timeoutTime <= currentTime) {
206 - scheduledTimeout(currentTime);
207 - timeoutTime = -1;
208 - scheduledTimeout = null;
209 - }
210 - unstable_flushExpired();
204 + if (scheduledTimeout !== null && timeoutTime <= currentTime) {
205 + scheduledTimeout(currentTime);
206 + timeoutTime = -1;
207 + scheduledTimeout = null;
208 }
209 }
210