diff --git a/packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js b/packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js index c0617f9b56787..6c8d677cdbb93 100644 --- a/packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js @@ -243,11 +243,12 @@ describe('ReactDOMNativeEventHeuristic-test', () => { mouseOverEvent.initEvent('mouseover', true, true); dispatchAndSetCurrentEvent(target.current, mouseOverEvent); - // 3s should be enough to expire the updates - Scheduler.unstable_advanceTime(3000); - expect(Scheduler).toFlushExpired([]); - expect(container.textContent).toEqual('hovered'); + // Flush discrete updates + ReactDOM.flushSync(); + // Since mouse over is not discrete, should not have updated yet + expect(container.textContent).toEqual('not hovered'); }); + expect(container.textContent).toEqual('hovered'); }); // @gate experimental @@ -275,11 +276,12 @@ describe('ReactDOMNativeEventHeuristic-test', () => { mouseEnterEvent.initEvent('mouseenter', true, true); dispatchAndSetCurrentEvent(target.current, mouseEnterEvent); - // 3s should be enough to expire the updates - Scheduler.unstable_advanceTime(3000); - expect(Scheduler).toFlushExpired([]); - expect(container.textContent).toEqual('hovered'); + // Flush discrete updates + ReactDOM.flushSync(); + // Since mouse end is not discrete, should not have updated yet + expect(container.textContent).toEqual('not hovered'); }); + expect(container.textContent).toEqual('hovered'); }); // @gate experimental diff --git a/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js index 9b7fed1468470..3eb9edd2fecbd 100644 --- a/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js +++ b/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js @@ -761,11 +761,12 @@ describe('ChangeEventPlugin', () => { mouseOverEvent.initEvent('mouseover', true, true); target.current.dispatchEvent(mouseOverEvent); - // 3s should be enough to expire the updates - Scheduler.unstable_advanceTime(3000); - expect(Scheduler).toFlushExpired([]); - expect(container.textContent).toEqual('hovered'); + // Flush discrete updates + ReactDOM.flushSync(); + // Since mouse enter/leave is not discrete, should not have updated yet + expect(container.textContent).toEqual('not hovered'); }); + expect(container.textContent).toEqual('hovered'); }); }); }); diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js index cd8cb58c6cedc..70f26e5fc4167 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js @@ -13,10 +13,7 @@ import type {ReactPriorityLevel} from './ReactInternalTypes'; // CommonJS interop named imports. import * as Scheduler from 'scheduler'; import {__interactionsRef} from 'scheduler/tracing'; -import { - enableSchedulerTracing, - enableSyncMicroTasks, -} from 'shared/ReactFeatureFlags'; +import {enableSchedulerTracing} from 'shared/ReactFeatureFlags'; import invariant from 'shared/invariant'; import { SyncLanePriority, @@ -139,7 +136,7 @@ export function scheduleSyncCallback(callback: SchedulerCallback) { // TODO: Figure out how to remove this It's only here as a last resort if we // forget to explicitly flush. - if (enableSyncMicroTasks && supportsMicrotasks) { + if (supportsMicrotasks) { // Flush the queue in a microtask. scheduleMicrotask(flushSyncCallbackQueueImpl); } else { diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js index 738ef2ea1afb7..3d0d0920fd22a 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js @@ -13,10 +13,7 @@ import type {ReactPriorityLevel} from './ReactInternalTypes'; // CommonJS interop named imports. import * as Scheduler from 'scheduler'; import {__interactionsRef} from 'scheduler/tracing'; -import { - enableSchedulerTracing, - enableSyncMicroTasks, -} from 'shared/ReactFeatureFlags'; +import {enableSchedulerTracing} from 'shared/ReactFeatureFlags'; import invariant from 'shared/invariant'; import { SyncLanePriority, @@ -139,7 +136,7 @@ export function scheduleSyncCallback(callback: SchedulerCallback) { // TODO: Figure out how to remove this It's only here as a last resort if we // forget to explicitly flush. - if (enableSyncMicroTasks && supportsMicrotasks) { + if (supportsMicrotasks) { // Flush the queue in a microtask. scheduleMicrotask(flushSyncCallbackQueueImpl); } else { diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index ed49965318212..cf8b539433e46 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -103,23 +103,32 @@ describe('ReactExpiration', () => { return {type: 'span', children: [], prop, hidden: false}; } + function flushNextRenderIfExpired() { + // This will start rendering the next level of work. If the work hasn't + // expired yet, React will exit without doing anything. If it has expired, + // it will schedule a sync task. + Scheduler.unstable_flushExpired(); + // Flush the sync task. + ReactNoop.flushSync(); + } + it('increases priority of updates as time progresses', () => { ReactNoop.render(); expect(ReactNoop.getChildren()).toEqual([]); // Nothing has expired yet because time hasn't advanced. - ReactNoop.flushExpired(); + flushNextRenderIfExpired(); expect(ReactNoop.getChildren()).toEqual([]); // Advance time a bit, but not enough to expire the low pri update. ReactNoop.expire(4500); - ReactNoop.flushExpired(); + flushNextRenderIfExpired(); expect(ReactNoop.getChildren()).toEqual([]); // Advance by another second. Now the update should expire and flush. - ReactNoop.expire(1000); - ReactNoop.flushExpired(); + ReactNoop.expire(500); + flushNextRenderIfExpired(); expect(ReactNoop.getChildren()).toEqual([span('done')]); }); @@ -323,7 +332,8 @@ describe('ReactExpiration', () => { Scheduler.unstable_advanceTime(10000); - expect(Scheduler).toFlushExpired(['D', 'E']); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded(['D', 'E']); expect(root).toMatchRenderedOutput('ABCDE'); }); @@ -351,7 +361,8 @@ describe('ReactExpiration', () => { Scheduler.unstable_advanceTime(10000); - expect(Scheduler).toFlushExpired(['D', 'E']); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded(['D', 'E']); expect(root).toMatchRenderedOutput('ABCDE'); }); @@ -373,12 +384,14 @@ describe('ReactExpiration', () => { ReactNoop.render('Hi'); // The update should not have expired yet. - expect(Scheduler).toFlushExpired([]); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded([]); expect(ReactNoop).toMatchRenderedOutput(null); // Advance the time some more to expire the update. Scheduler.unstable_advanceTime(10000); - expect(Scheduler).toFlushExpired([]); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded([]); expect(ReactNoop).toMatchRenderedOutput('Hi'); }); @@ -391,7 +404,8 @@ describe('ReactExpiration', () => { Scheduler.unstable_advanceTime(10000); ReactNoop.render('Hi'); - expect(Scheduler).toFlushExpired([]); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded([]); expect(ReactNoop).toMatchRenderedOutput(null); // Advancing by ~5 seconds should be sufficient to expire the update. (I @@ -399,7 +413,8 @@ describe('ReactExpiration', () => { Scheduler.unstable_advanceTime(6000); ReactNoop.render('Hi'); - expect(Scheduler).toFlushExpired([]); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded([]); expect(ReactNoop).toMatchRenderedOutput('Hi'); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index a14c36bfce2bb..df691e074b2a4 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -402,7 +402,7 @@ describe('ReactIncrementalErrorHandling', () => { expect(ReactNoop.getChildren()).toEqual([]); }); - it('retries one more time if an error occurs during a render that expires midway through the tree', () => { + it('retries one more time if an error occurs during a render that expires midway through the tree', async () => { function Oops({unused}) { Scheduler.unstable_yieldValue('Oops'); throw new Error('Oops'); @@ -432,7 +432,11 @@ describe('ReactIncrementalErrorHandling', () => { // Expire the render midway through Scheduler.unstable_advanceTime(10000); - expect(() => Scheduler.unstable_flushExpired()).toThrow('Oops'); + + expect(() => { + Scheduler.unstable_flushExpired(); + ReactNoop.flushSync(); + }).toThrow('Oops'); expect(Scheduler).toHaveYielded([ // The render expired, but we shouldn't throw out the partial work. diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js index 95acb7d8fc778..ec587d7231c78 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js @@ -32,6 +32,15 @@ describe('ReactIncrementalUpdates', () => { return {type: 'span', children: [], prop, hidden: false}; } + function flushNextRenderIfExpired() { + // This will start rendering the next level of work. If the work hasn't + // expired yet, React will exit without doing anything. If it has expired, + // it will schedule a sync task. + Scheduler.unstable_flushExpired(); + // Flush the sync task. + ReactNoop.flushSync(); + } + it('applies updates in order of priority', () => { let state; class Foo extends React.Component { @@ -469,7 +478,8 @@ describe('ReactIncrementalUpdates', () => { ReactNoop.act(() => { ReactNoop.render(); - expect(Scheduler).toFlushExpired([]); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded([]); expect(Scheduler).toFlushAndYield([ 'Render: 0', 'Commit: 0', @@ -479,7 +489,8 @@ describe('ReactIncrementalUpdates', () => { Scheduler.unstable_advanceTime(10000); setCount(2); - expect(Scheduler).toFlushExpired([]); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded([]); }); }); @@ -497,7 +508,8 @@ describe('ReactIncrementalUpdates', () => { Scheduler.unstable_advanceTime(10000); ReactNoop.render(); - expect(Scheduler).toFlushExpired([]); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded([]); }); it('regression: does not expire soon due to previous expired work', () => { @@ -508,12 +520,14 @@ describe('ReactIncrementalUpdates', () => { ReactNoop.render(); Scheduler.unstable_advanceTime(10000); - expect(Scheduler).toFlushExpired(['A']); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded(['A']); Scheduler.unstable_advanceTime(10000); ReactNoop.render(); - expect(Scheduler).toFlushExpired([]); + flushNextRenderIfExpired(); + expect(Scheduler).toHaveYielded([]); }); it('when rebasing, does not exclude updates that were already committed, regardless of priority', async () => { diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index d7da84e67b752..692b745f97085 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -984,6 +984,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('C')]); }); + // TODO: This test was written against the old Expiration Times + // implementation. It doesn't really test what it was intended to test + // anymore, because all updates to the same queue get entangled together. + // Even if they haven't expired. Consider either deleting or rewriting. // @gate enableCache it('flushes all expired updates in a single batch', async () => { class Foo extends React.Component { @@ -1013,10 +1017,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { jest.advanceTimersByTime(1000); ReactNoop.render(); - Scheduler.unstable_advanceTime(10000); - jest.advanceTimersByTime(10000); - - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushAndYield([ 'Suspend! [goodbye]', 'Loading...', 'Commit: goodbye', @@ -1797,12 +1798,32 @@ describe('ReactSuspenseWithNoopRenderer', () => { await advanceTimers(5000); // Retry with the new content. - expect(Scheduler).toFlushAndYield([ - 'A', - // B still suspends - 'Suspend! [B]', - 'Loading more...', - ]); + if (gate(flags => flags.disableSchedulerTimeoutInWorkLoop)) { + expect(Scheduler).toFlushAndYield([ + 'A', + // B still suspends + 'Suspend! [B]', + 'Loading more...', + ]); + } else { + // In this branch, right as we start rendering, we detect that the work + // has expired (via Scheduler's didTimeout argument) and re-schedule the + // work as synchronous. Since sync work does not flow through Scheduler, + // we need to use `flushSync`. + // + // Usually we would use `act`, which fluses both sync work and Scheduler + // work, but that would also force the fallback to display, and this test + // is specifically about whether we delay or show the fallback. + expect(Scheduler).toFlushAndYield([]); + // This will flush the synchronous callback we just scheduled. + ReactNoop.flushSync(); + expect(Scheduler).toHaveYielded([ + 'A', + // B still suspends + 'Suspend! [B]', + 'Loading more...', + ]); + } // Because we've already been waiting for so long we've exceeded // our threshold and we show the next level immediately. expect(ReactNoop.getChildren()).toEqual([ diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 75ab4b6842d90..77079bba3a6a1 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -150,6 +150,4 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; -export const enableSyncMicroTasks = false; - export const enableLazyContextPropagation = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index b6a8016ec8b77..2e2f64e4bc5d0 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -57,7 +57,6 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; -export const enableSyncMicroTasks = false; export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index cb8721af25fe9..34ee7d2b7a843 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; -export const enableSyncMicroTasks = false; export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index b504491d98de2..bd5e91885ca44 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; -export const enableSyncMicroTasks = false; export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index ddab49284e437..70c5dc4144ee8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; -export const enableSyncMicroTasks = false; export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 7098bfed2aa5b..df6bbdf03fbed 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; -export const enableSyncMicroTasks = false; export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index a321f5ea12f92..42003be9a388b 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; -export const enableSyncMicroTasks = false; export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index bb6a952355657..5ab680976a9e4 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false; export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; -export const enableSyncMicroTasks = false; export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index df98012634970..e2496510ef24c 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -55,5 +55,4 @@ export const enableUseRefAccessWarning = __VARIANT__; export const enableProfilerNestedUpdateScheduledHook = __VARIANT__; export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; -export const enableSyncMicroTasks = __VARIANT__; export const enableLazyContextPropagation = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 893c2699b0d07..c29856558751e 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -31,7 +31,6 @@ export const { enableUseRefAccessWarning, disableNativeComponentFrames, disableSchedulerTimeoutInWorkLoop, - enableSyncMicroTasks, enableLazyContextPropagation, } = dynamicFeatureFlags;