From 9377006bfeb2fa280c13312dd546cdd56d0b747d Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Wed, 10 Mar 2021 14:56:25 -0700 Subject: [PATCH] Remove flushExpired --- .../__tests__/ReactSuspense-test.internal.js | 59 ++++++++++--------- .../__tests__/ReactProfiler-test.internal.js | 6 +- ...ofilerDevToolsIntegration-test.internal.js | 2 +- .../src/__tests__/SchedulerMock-test.js | 51 ++-------------- packages/scheduler/src/forks/SchedulerMock.js | 19 +----- .../jest/matchers/schedulerTestMatchers.js | 10 ---- 6 files changed, 42 insertions(+), 105 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 6e724fd0a5679..9a9f57ccd20b4 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -370,7 +370,7 @@ describe('ReactSuspense', () => { await LazyClass; - expect(Scheduler).toFlushExpired(['Hi', 'Did mount: Hi']); + expect(Scheduler).toFlushUntilNextPaint(['Hi', 'Did mount: Hi']); expect(root).toMatchRenderedOutput('Hi'); }); @@ -729,7 +729,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(100); expect(Scheduler).toHaveYielded(['Promise resolved [B:1]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ 'B:1', 'Unmount [Loading...]', // Should be a mount, not an update @@ -748,7 +748,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(100); expect(Scheduler).toHaveYielded(['Promise resolved [B:2]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ 'B:2', 'Unmount [Loading...]', 'Update [B:2]', @@ -786,7 +786,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); - expect(Scheduler).toFlushExpired(['A']); + expect(Scheduler).toFlushUntilNextPaint(['A']); expect(root).toMatchRenderedOutput('Stateful: 1A'); root.update(); @@ -804,7 +804,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); - expect(Scheduler).toFlushExpired(['B']); + expect(Scheduler).toFlushUntilNextPaint(['B']); expect(root).toMatchRenderedOutput('Stateful: 2B'); }); @@ -846,7 +846,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); - expect(Scheduler).toFlushExpired(['A']); + expect(Scheduler).toFlushUntilNextPaint(['A']); expect(root).toMatchRenderedOutput('Stateful: 1A'); root.update(); @@ -871,7 +871,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); - expect(Scheduler).toFlushExpired(['B']); + expect(Scheduler).toFlushUntilNextPaint(['B']); expect(root).toMatchRenderedOutput('Stateful: 2B'); }); @@ -951,7 +951,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(500); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); - expect(Scheduler).toFlushExpired(['A', 'Did commit: A']); + expect(Scheduler).toFlushUntilNextPaint(['A', 'Did commit: A']); }); it('retries when an update is scheduled on a timed out tree', () => { @@ -1038,7 +1038,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Child 1]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ 'Child 1', 'Suspend! [Child 2]', 'Suspend! [Child 3]', @@ -1047,12 +1047,15 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Child 2]']); - expect(Scheduler).toFlushExpired(['Child 2', 'Suspend! [Child 3]']); + expect(Scheduler).toFlushUntilNextPaint([ + 'Child 2', + 'Suspend! [Child 3]', + ]); jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Child 3]']); - expect(Scheduler).toFlushExpired(['Child 3']); + expect(Scheduler).toFlushUntilNextPaint(['Child 3']); expect(root).toMatchRenderedOutput( ['Child 1', 'Child 2', 'Child 3'].join(''), ); @@ -1113,7 +1116,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Tab: 0]']); - expect(Scheduler).toFlushExpired(['Tab: 0']); + expect(Scheduler).toFlushUntilNextPaint(['Tab: 0']); expect(root).toMatchRenderedOutput('Tab: 0 + sibling'); act(() => setTab(1)); @@ -1126,7 +1129,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Tab: 1]']); - expect(Scheduler).toFlushExpired(['Tab: 1']); + expect(Scheduler).toFlushUntilNextPaint(['Tab: 1']); expect(root).toMatchRenderedOutput('Tab: 1 + sibling'); act(() => setTab(2)); @@ -1139,7 +1142,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Tab: 2]']); - expect(Scheduler).toFlushExpired(['Tab: 2']); + expect(Scheduler).toFlushUntilNextPaint(['Tab: 2']); expect(root).toMatchRenderedOutput('Tab: 2 + sibling'); }); @@ -1177,7 +1180,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [A:0]']); - expect(Scheduler).toFlushExpired(['A:0']); + expect(Scheduler).toFlushUntilNextPaint(['A:0']); expect(root).toMatchRenderedOutput('A:0'); act(() => setStep(1)); @@ -1215,7 +1218,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ 'A', // The promises for B and C have now been thrown twice 'Suspend! [B]', @@ -1226,7 +1229,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ // Even though the promise for B was thrown twice, we should only // re-render once. 'B', @@ -1238,7 +1241,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [C]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ // Even though the promise for C was thrown three times, we should only // re-render once. 'C', @@ -1280,7 +1283,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [A]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ 'A', // The promises for B and C have now been thrown twice 'Suspend! [B]', @@ -1291,7 +1294,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [B]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ // Even though the promise for B was thrown twice, we should only // re-render once. 'B', @@ -1393,7 +1396,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [default]']); - expect(Scheduler).toFlushExpired(['default']); + expect(Scheduler).toFlushUntilNextPaint(['default']); expect(root).toMatchRenderedOutput('default'); act(() => setValue('new value')); @@ -1401,7 +1404,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [new value]']); - expect(Scheduler).toFlushExpired(['new value']); + expect(Scheduler).toFlushUntilNextPaint(['new value']); expect(root).toMatchRenderedOutput('new value'); }); @@ -1450,7 +1453,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [default]']); - expect(Scheduler).toFlushExpired(['default']); + expect(Scheduler).toFlushUntilNextPaint(['default']); expect(root).toMatchRenderedOutput('default'); act(() => setValue('new value')); @@ -1458,7 +1461,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [new value]']); - expect(Scheduler).toFlushExpired(['new value']); + expect(Scheduler).toFlushUntilNextPaint(['new value']); expect(root).toMatchRenderedOutput('new value'); }); @@ -1506,7 +1509,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [default]']); - expect(Scheduler).toFlushExpired(['default']); + expect(Scheduler).toFlushUntilNextPaint(['default']); expect(root).toMatchRenderedOutput('default'); act(() => setValue('new value')); @@ -1514,7 +1517,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [new value]']); - expect(Scheduler).toFlushExpired(['new value']); + expect(Scheduler).toFlushUntilNextPaint(['new value']); expect(root).toMatchRenderedOutput('new value'); }); @@ -1558,7 +1561,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [default]']); - expect(Scheduler).toFlushExpired(['default']); + expect(Scheduler).toFlushUntilNextPaint(['default']); expect(root).toMatchRenderedOutput('default'); act(() => setValue('new value')); @@ -1566,7 +1569,7 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [new value]']); - expect(Scheduler).toFlushExpired(['new value']); + expect(Scheduler).toFlushUntilNextPaint(['new value']); expect(root).toMatchRenderedOutput('new value'); }); }); diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 93347b1ac2f9f..ff2897141b57f 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -4306,7 +4306,7 @@ describe('Profiler', () => { await resourcePromise; expect(Scheduler).toHaveYielded(['Promise resolved [loaded]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushUntilNextPaint([ 'onPostCommit', 'AsyncText [loaded]', ]); @@ -4370,7 +4370,7 @@ describe('Profiler', () => { await resourcePromise; expect(Scheduler).toHaveYielded(['Promise resolved [loaded]']); - expect(Scheduler).toFlushExpired(['onPostCommit', 'render']); + expect(Scheduler).toFlushUntilNextPaint(['onPostCommit', 'render']); expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); @@ -4575,7 +4575,7 @@ describe('Profiler', () => { await originalPromise; expect(Scheduler).toHaveYielded(['Promise resolved [loaded]']); - expect(Scheduler).toFlushExpired(['AsyncText [loaded]']); + expect(Scheduler).toFlushUntilNextPaint(['AsyncText [loaded]']); expect(renderer.toJSON()).toEqual(['loaded', 'updated']); expect(Scheduler).toFlushAndYield(['onPostCommit']); diff --git a/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js b/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js index d2f52dea1b11a..33454da6649d2 100644 --- a/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js +++ b/packages/react/src/__tests__/ReactProfilerDevToolsIntegration-test.internal.js @@ -198,7 +198,7 @@ describe('ReactProfiler DevTools integration', () => { root.update(); // Update B should not instantly expire. - expect(Scheduler).toFlushExpired([]); + expect(Scheduler).toFlushAndYieldThrough([]); expect(Scheduler).toFlushAndYield(['B']); expect(root).toMatchRenderedOutput('B'); diff --git a/packages/scheduler/src/__tests__/SchedulerMock-test.js b/packages/scheduler/src/__tests__/SchedulerMock-test.js index e8da8ee53d69a..5b7303dc685cd 100644 --- a/packages/scheduler/src/__tests__/SchedulerMock-test.js +++ b/packages/scheduler/src/__tests__/SchedulerMock-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).toFlushExpired([ + expect(Scheduler).toFlushAndYieldThrough([ 'B (did timeout: true)', 'C (did timeout: true)', ]); // Expire A Scheduler.unstable_advanceTime(4600); - expect(Scheduler).toFlushExpired(['A (did timeout: true)']); + expect(Scheduler).toFlushAndYieldThrough(['A (did timeout: true)']); // Flush the rest without expiring expect(Scheduler).toFlushAndYield([ @@ -133,16 +133,6 @@ describe('Scheduler', () => { ]); }); - it('has a default expiration of ~5 seconds', () => { - scheduleCallback(NormalPriority, () => Scheduler.unstable_yieldValue('A')); - - Scheduler.unstable_advanceTime(4999); - expect(Scheduler).toHaveYielded([]); - - Scheduler.unstable_advanceTime(1); - expect(Scheduler).toFlushExpired(['A']); - }); - it('continues working on same task after yielding', () => { scheduleCallback(NormalPriority, () => { Scheduler.unstable_advanceTime(100); @@ -191,35 +181,6 @@ describe('Scheduler', () => { expect(Scheduler).toFlushAndYield(['C2', 'C3', 'D', 'E']); }); - it('continuation callbacks inherit the expiration of the previous callback', () => { - const tasks = [ - ['A', 125], - ['B', 124], - ['C', 100], - ['D', 100], - ]; - const work = () => { - while (tasks.length > 0) { - const [label, ms] = tasks.shift(); - Scheduler.unstable_advanceTime(ms); - Scheduler.unstable_yieldValue(label); - if (shouldYield()) { - return work; - } - } - }; - - // Schedule a high priority callback - scheduleCallback(UserBlockingPriority, work); - - // Flush until just before the expiration time - expect(Scheduler).toFlushAndYieldThrough(['A', 'B']); - - // Advance time by just a bit more. This should expire all the remaining work. - Scheduler.unstable_advanceTime(1); - expect(Scheduler).toFlushExpired(['C', 'D']); - }); - it('continuations are interrupted by higher priority work', () => { const tasks = [ ['A', 100], @@ -322,7 +283,7 @@ describe('Scheduler', () => { // Immediate callback hasn't fired, yet. expect(Scheduler).toHaveYielded([]); // They all flush immediately within the subsequent task. - expect(Scheduler).toFlushExpired(['A', 'B', 'C', 'D']); + expect(Scheduler).toFlushUntilNextPaint(['A', 'B', 'C', 'D']); }); it('nested immediate callbacks are added to the queue of immediate callbacks', () => { @@ -341,7 +302,7 @@ describe('Scheduler', () => { ); expect(Scheduler).toHaveYielded([]); // C should flush at the end - expect(Scheduler).toFlushExpired(['A', 'B', 'D', 'C']); + expect(Scheduler).toFlushUntilNextPaint(['A', 'B', 'D', 'C']); }); it('wrapped callbacks have same signature as original callback', () => { @@ -406,12 +367,12 @@ describe('Scheduler', () => { throw new Error('Oops C'); }); - expect(() => expect(Scheduler).toFlushExpired()).toThrow('Oops A'); + expect(() => expect(Scheduler).toFlushUntilNextPaint()).toThrow('Oops A'); expect(Scheduler).toHaveYielded(['A']); // B and C flush in a subsequent event. That way, the second error is not // swallowed. - expect(() => expect(Scheduler).toFlushExpired()).toThrow('Oops C'); + expect(() => expect(Scheduler).toFlushUntilNextPaint()).toThrow('Oops C'); expect(Scheduler).toHaveYielded(['B', 'C']); }); diff --git a/packages/scheduler/src/forks/SchedulerMock.js b/packages/scheduler/src/forks/SchedulerMock.js index ef72933db563d..ece9ed7227c8e 100644 --- a/packages/scheduler/src/forks/SchedulerMock.js +++ b/packages/scheduler/src/forks/SchedulerMock.js @@ -414,6 +414,7 @@ function cancelHostTimeout(): void { function shouldYieldToHost(): boolean { if ( + (expectedNumberOfYields === 0 && yieldedValues === null) || (expectedNumberOfYields !== -1 && yieldedValues !== null && yieldedValues.length >= expectedNumberOfYields) || @@ -499,23 +500,6 @@ function unstable_flushUntilNextPaint(): void { } } -function unstable_flushExpired() { - if (isFlushing) { - throw new Error('Already flushing work.'); - } - if (scheduledCallback !== null) { - isFlushing = true; - try { - const hasMoreWork = scheduledCallback(false, currentMockTime); - if (!hasMoreWork) { - scheduledCallback = null; - } - } finally { - isFlushing = false; - } - } -} - function unstable_flushAllWithoutAsserting(): boolean { // Returns false if no work was flushed. if (isFlushing) { @@ -621,7 +605,6 @@ export { forceFrameRate as unstable_forceFrameRate, unstable_flushAllWithoutAsserting, unstable_flushNumberOfYields, - unstable_flushExpired, unstable_clearYields, unstable_flushUntilNextPaint, unstable_flushAll, diff --git a/scripts/jest/matchers/schedulerTestMatchers.js b/scripts/jest/matchers/schedulerTestMatchers.js index f18ccfc548093..47f3947d67e71 100644 --- a/scripts/jest/matchers/schedulerTestMatchers.js +++ b/scripts/jest/matchers/schedulerTestMatchers.js @@ -57,15 +57,6 @@ function toFlushWithoutYielding(Scheduler) { return toFlushAndYield(Scheduler, []); } -function toFlushExpired(Scheduler, expectedYields) { - assertYieldsWereCleared(Scheduler); - Scheduler.unstable_flushExpired(); - const actualYields = Scheduler.unstable_clearYields(); - return captureAssertion(() => { - expect(actualYields).toEqual(expectedYields); - }); -} - function toHaveYielded(Scheduler, expectedYields) { return captureAssertion(() => { const actualYields = Scheduler.unstable_clearYields(); @@ -87,7 +78,6 @@ module.exports = { toFlushAndYieldThrough, toFlushUntilNextPaint, toFlushWithoutYielding, - toFlushExpired, toHaveYielded, toFlushAndThrow, };