From 4b63f61e188e9edb1a774f5decdbd8a3aa6ebe12 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Tue, 23 Feb 2021 23:28:23 -0700 Subject: [PATCH 1/3] Schedule sync updates in microtask --- ...DOMServerPartialHydration-test.internal.js | 1 + .../ReactServerRenderingHydration-test.js | 1 + .../src/SchedulerWithReactIntegration.new.js | 13 +++-- .../src/SchedulerWithReactIntegration.old.js | 13 +++-- .../src/__tests__/ReactExpiration-test.js | 11 ++-- .../ReactHooksWithNoopRenderer-test.js | 20 +++++-- .../src/__tests__/ReactOffscreen-test.js | 12 ++++- .../ReactSchedulerIntegration-test.js | 14 +++-- .../src/__tests__/ReactSuspenseList-test.js | 12 +++-- .../ReactSuspensePlaceholder-test.internal.js | 23 ++++++-- .../ReactSuspenseWithNoopRenderer-test.js | 54 ++++++++++++------- .../useMutableSource-test.internal.js | 11 ++-- 12 files changed, 131 insertions(+), 54 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 67d1a29f0ab03..2ad221d5fbcb6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -380,6 +380,7 @@ describe('ReactDOMServerPartialHydration', () => { resolve(); await promise; Scheduler.unstable_flushAll(); + await null; jest.runAllTimers(); // We should now have hydrated with a ref on the existing span. diff --git a/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js b/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js index fea9d79b4735d..a361e55a7f8fc 100644 --- a/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js @@ -488,6 +488,7 @@ describe('ReactDOMServerHydration', () => { jest.runAllTimers(); await Promise.resolve(); Scheduler.unstable_flushAll(); + await null; expect(element.textContent).toBe('Hello world'); }); diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js index 36dbc704fc7cd..9b7b939bcf912 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js @@ -23,6 +23,7 @@ import { getCurrentUpdateLanePriority, setCurrentUpdateLanePriority, } from './ReactFiberLane.new'; +import {scheduleMicrotask, supportsMicrotasks} from './ReactFiberHostConfig'; const { unstable_runWithPriority: Scheduler_runWithPriority, @@ -147,10 +148,14 @@ export function scheduleSyncCallback(callback: SchedulerCallback) { // Flush the queue in the next tick, at the earliest. // TODO: Figure out how to remove this It's only here as a last resort if we // forget to explicitly flush. - immediateQueueCallbackNode = Scheduler_scheduleCallback( - Scheduler_ImmediatePriority, - flushSyncCallbackQueueImpl, - ); + if (supportsMicrotasks) { + scheduleMicrotask(flushSyncCallbackQueueImpl); + } else { + immediateQueueCallbackNode = Scheduler_scheduleCallback( + Scheduler_ImmediatePriority, + flushSyncCallbackQueueImpl, + ); + } } else { // Push onto existing queue. Don't need to schedule a callback because // we already scheduled one when we created the queue. diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js index 1d309df4020f1..51961a4ebd2b4 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js @@ -23,6 +23,7 @@ import { getCurrentUpdateLanePriority, setCurrentUpdateLanePriority, } from './ReactFiberLane.old'; +import {scheduleMicrotask, supportsMicrotasks} from './ReactFiberHostConfig'; const { unstable_runWithPriority: Scheduler_runWithPriority, @@ -147,10 +148,14 @@ export function scheduleSyncCallback(callback: SchedulerCallback) { // Flush the queue in the next tick, at the earliest. // TODO: Figure out how to remove this It's only here as a last resort if we // forget to explicitly flush. - immediateQueueCallbackNode = Scheduler_scheduleCallback( - Scheduler_ImmediatePriority, - flushSyncCallbackQueueImpl, - ); + if (supportsMicrotasks) { + scheduleMicrotask(flushSyncCallbackQueueImpl); + } else { + immediateQueueCallbackNode = Scheduler_scheduleCallback( + Scheduler_ImmediatePriority, + flushSyncCallbackQueueImpl, + ); + } } else { // Push onto existing queue. Don't need to schedule a callback because // we already scheduled one when we created the queue. diff --git a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js index 017c712bd814b..48ff50880f943 100644 --- a/packages/react-reconciler/src/__tests__/ReactExpiration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactExpiration-test.js @@ -598,12 +598,13 @@ describe('ReactExpiration', () => { // second one. Scheduler.unstable_advanceTime(1000); // Attempt to interrupt with a high pri update. - updateHighPri(); + await ReactNoop.act(async () => { + updateHighPri(); + }); - // The first update expired, so first will finish it without - // interrupting. But not the second update, which hasn't expired yet. - expect(Scheduler).toFlushExpired(['Sibling']); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ + // The first update expired + 'Sibling', // Then render the high pri update 'High pri: 1', 'Normal pri: 1', diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index bf5503feb4902..4e42e60ebac94 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -1792,7 +1792,7 @@ describe('ReactHooksWithNoopRenderer', () => { it( 'in legacy mode, useEffect is deferred and updates finish synchronously ' + '(in a single batch)', - () => { + async () => { function Counter(props) { const [count, updateCount] = useState('(empty)'); useEffect(() => { @@ -1807,11 +1807,21 @@ describe('ReactHooksWithNoopRenderer', () => { }, [props.count]); return ; } - act(() => { + await act(async () => { ReactNoop.renderLegacySyncRoot(); - // Even in legacy mode, effects are deferred until after paint - expect(Scheduler).toFlushAndYieldThrough(['Count: (empty)']); - expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); + + if (gate(flags => flags.enableDiscreteEventMicroTasks)) { + // Flush microtasks. + await null; + + // Even in legacy mode, effects are deferred until after paint + expect(Scheduler).toHaveYielded(['Count: (empty)']); + expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); + } else { + // Even in legacy mode, effects are deferred until after paint + expect(Scheduler).toFlushAndYieldThrough(['Count: (empty)']); + expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); + } }); // effects get forced on exiting act() diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index e830fa2b24ddf..bbd55f168ff42 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -98,8 +98,16 @@ describe('ReactOffscreen', () => { , ); - // Should not defer the hidden tree - expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']); + if (gate(flags => flags.enableDiscreteEventMicroTasks)) { + // Flush microtasks. + await null; + + // Should not defer the hidden tree + expect(Scheduler).toHaveYielded(['A', 'Outside']); + } else { + // Should not defer the hidden tree + expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']); + } }); expect(root).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js index 4292333293e82..a1ebb3065c957 100644 --- a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js @@ -569,9 +569,17 @@ describe( ReactNoop.render(); }); - // Because the render expired, React should finish the tree without - // consulting `shouldYield` again - expect(Scheduler).toFlushExpired(['B', 'C']); + if (gate(flags => flags.enableDiscreteEventMicroTasks)) { + await null; + + // Because the render expired, React should finish the tree without + // consulting `shouldYield` again + expect(Scheduler).toHaveYielded(['B', 'C']); + } else { + // Because the render expired, React should finish the tree without + // consulting `shouldYield` again + expect(Scheduler).toFlushExpired(['B', 'C']); + } }); }); }, diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index de74c18e5ca6a..2cdc2621fbe9a 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -292,9 +292,11 @@ describe('ReactSuspenseList', () => { , ); - await C.resolve(); + await ReactNoop.act(async () => { + C.resolve(); + }); - expect(Scheduler).toFlushAndYield(['C']); + expect(Scheduler).toHaveYielded(['C']); expect(ReactNoop).toMatchRenderedOutput( <> @@ -304,9 +306,11 @@ describe('ReactSuspenseList', () => { , ); - await B.resolve(); + await ReactNoop.act(async () => { + B.resolve(); + }); - expect(Scheduler).toFlushAndYield(['B']); + expect(Scheduler).toHaveYielded(['B']); expect(ReactNoop).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index d40f56380b7e8..15f7eba92e51b 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -310,7 +310,7 @@ describe('ReactSuspensePlaceholder', () => { }); describe('when suspending during mount', () => { - it('properly accounts for base durations when a suspended times out in a legacy tree', () => { + it('properly accounts for base durations when a suspended times out in a legacy tree', async () => { ReactNoop.renderLegacySyncRoot(); expect(Scheduler).toHaveYielded([ 'App', @@ -331,7 +331,14 @@ describe('ReactSuspensePlaceholder', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); - expect(Scheduler).toFlushExpired(['Loaded']); + if (gate(flags => flags.enableDiscreteEventMicroTasks)) { + // Flush microtasks + await null; + + expect(Scheduler).toHaveYielded(['Loaded']); + } else { + expect(Scheduler).toFlushExpired(['Loaded']); + } expect(ReactNoop).toMatchRenderedOutput('LoadedText'); expect(onRender).toHaveBeenCalledTimes(2); @@ -378,7 +385,7 @@ describe('ReactSuspensePlaceholder', () => { }); describe('when suspending during update', () => { - it('properly accounts for base durations when a suspended times out in a legacy tree', () => { + it('properly accounts for base durations when a suspended times out in a legacy tree', async () => { ReactNoop.renderLegacySyncRoot( , ); @@ -427,7 +434,15 @@ describe('ReactSuspensePlaceholder', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); - expect(Scheduler).toFlushExpired(['Loaded']); + + if (gate(flags => flags.enableDiscreteEventMicroTasks)) { + // Flush microtasks + await null; + + expect(Scheduler).toHaveYielded(['Loaded']); + } else { + expect(Scheduler).toFlushExpired(['Loaded']); + } expect(ReactNoop).toMatchRenderedOutput('LoadedNew'); expect(onRender).toHaveBeenCalledTimes(4); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 20971e9f58e91..8e8a512dacf41 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -1088,9 +1088,11 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(Scheduler).toHaveYielded(['Suspend! [Result]', 'Loading...']); expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); - await resolveText('Result'); + await ReactNoop.act(async () => { + resolveText('Result'); + }); - expect(Scheduler).toFlushExpired(['Result']); + expect(Scheduler).toHaveYielded(['Result']); expect(ReactNoop.getChildren()).toEqual([span('Result')]); }); @@ -1156,8 +1158,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { , ); - await resolveText('Step: 2'); - expect(Scheduler).toFlushExpired(['Step: 2']); + await ReactNoop.act(async () => { + resolveText('Step: 2'); + }); + expect(Scheduler).toHaveYielded(['Step: 2']); expect(ReactNoop).toMatchRenderedOutput( <> @@ -1227,9 +1231,11 @@ describe('ReactSuspenseWithNoopRenderer', () => { , ); - await resolveText('B'); + await ReactNoop.act(async () => { + resolveText('B'); + }); - expect(Scheduler).toFlushExpired(['B']); + expect(Scheduler).toHaveYielded(['B']); expect(ReactNoop).toMatchRenderedOutput( <> @@ -1271,9 +1277,11 @@ describe('ReactSuspenseWithNoopRenderer', () => { ]); expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); - await resolveText('Hi'); + await ReactNoop.act(async () => { + resolveText('Hi'); + }); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toHaveYielded([ 'constructor', 'Hi', 'componentDidMount', @@ -1316,8 +1324,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Loading...', ]); expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); - await resolveText('Hi'); - expect(Scheduler).toFlushExpired(['Hi']); + await ReactNoop.act(async () => { + resolveText('Hi'); + }); + expect(Scheduler).toHaveYielded(['Hi']); expect(ReactNoop.getChildren()).toEqual([span('Hi')]); }); @@ -1360,8 +1370,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { , ]); - await resolveText('Hi'); - expect(Scheduler).toFlushExpired(['Hi']); + await ReactNoop.act(async () => { + resolveText('Hi'); + }); + expect(Scheduler).toHaveYielded(['Hi']); }); } else { // @gate enableCache @@ -1401,9 +1413,11 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Child is hidden: true', ]); - await resolveText('Hi'); + await ReactNoop.act(async () => { + resolveText('Hi'); + }); - expect(Scheduler).toFlushExpired(['Hi']); + expect(Scheduler).toHaveYielded(['Hi']); }); } @@ -1647,9 +1661,11 @@ describe('ReactSuspenseWithNoopRenderer', () => { , ); - await resolveText('B'); + await ReactNoop.act(async () => { + resolveText('B'); + }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'B', 'Destroy Layout Effect [Loading...]', 'Layout Effect [B]', @@ -1681,9 +1697,11 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Effect [Loading...]', ]); - await resolveText('B2'); + await ReactNoop.act(async () => { + resolveText('B2'); + }); - expect(Scheduler).toFlushAndYield([ + expect(Scheduler).toHaveYielded([ 'B2', 'Destroy Layout Effect [Loading...]', 'Destroy Layout Effect [B]', diff --git a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js index db4214bfd0656..cf1db2891e11d 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js +++ b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js @@ -1794,7 +1794,7 @@ describe('useMutableSource', () => { }); // @gate experimental - it('should not misidentify mutations after render as side effects', () => { + it('should not misidentify mutations after render as side effects', async () => { const source = createSource('initial'); const mutableSource = createMutableSource( source, @@ -1811,15 +1811,16 @@ describe('useMutableSource', () => { return null; } - act(() => { + await act(async () => { ReactNoop.renderLegacySyncRoot( , ); - expect(Scheduler).toFlushAndYieldThrough([ - 'MutateDuringRead:initial', - ]); + }); + expect(Scheduler).toHaveYielded(['MutateDuringRead:initial']); + + await act(async () => { source.value = 'updated'; }); expect(Scheduler).toHaveYielded(['MutateDuringRead:updated']); From 4cea1e04126eefbf298746481afb3cc5f73e7833 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Thu, 25 Feb 2021 14:22:49 -0700 Subject: [PATCH 2/3] Updates from review --- .../src/SchedulerWithReactIntegration.new.js | 10 +++++---- .../src/SchedulerWithReactIntegration.old.js | 10 +++++---- .../ReactHooksWithNoopRenderer-test.js | 16 ++++---------- ...tIncrementalErrorHandling-test.internal.js | 4 ++++ .../src/__tests__/ReactOffscreen-test.js | 13 ++++-------- .../ReactSchedulerIntegration-test.js | 16 +++++--------- .../ReactSuspensePlaceholder-test.internal.js | 21 ++++++------------- packages/shared/ReactFeatureFlags.js | 2 ++ .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 17 files changed, 46 insertions(+), 55 deletions(-) diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js index 9b7b939bcf912..8d4f4c553a227 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js @@ -16,6 +16,7 @@ import {__interactionsRef} from 'scheduler/tracing'; import { enableSchedulerTracing, decoupleUpdatePriorityFromScheduler, + enableSyncMicroTasks, } from 'shared/ReactFeatureFlags'; import invariant from 'shared/invariant'; import { @@ -145,12 +146,13 @@ export function scheduleSyncCallback(callback: SchedulerCallback) { // the next tick, or earlier if something calls `flushSyncCallbackQueue`. if (syncQueue === null) { syncQueue = [callback]; - // Flush the queue in the next tick, at the earliest. - // TODO: Figure out how to remove this It's only here as a last resort if we - // forget to explicitly flush. - if (supportsMicrotasks) { + if (enableSyncMicroTasks && supportsMicrotasks) { + // Flush the queue in a microtask. scheduleMicrotask(flushSyncCallbackQueueImpl); } else { + // Flush the queue in the next tick, at the earliest. + // TODO: Figure out how to remove this It's only here as a last resort if we + // forget to explicitly flush. immediateQueueCallbackNode = Scheduler_scheduleCallback( Scheduler_ImmediatePriority, flushSyncCallbackQueueImpl, diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js index 51961a4ebd2b4..96a1077b380df 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js @@ -16,6 +16,7 @@ import {__interactionsRef} from 'scheduler/tracing'; import { enableSchedulerTracing, decoupleUpdatePriorityFromScheduler, + enableSyncMicroTasks, } from 'shared/ReactFeatureFlags'; import invariant from 'shared/invariant'; import { @@ -145,12 +146,13 @@ export function scheduleSyncCallback(callback: SchedulerCallback) { // the next tick, or earlier if something calls `flushSyncCallbackQueue`. if (syncQueue === null) { syncQueue = [callback]; - // Flush the queue in the next tick, at the earliest. - // TODO: Figure out how to remove this It's only here as a last resort if we - // forget to explicitly flush. - if (supportsMicrotasks) { + if (enableSyncMicroTasks && supportsMicrotasks) { + // Flush the queue in a microtask. scheduleMicrotask(flushSyncCallbackQueueImpl); } else { + // Flush the queue in the next tick, at the earliest. + // TODO: Figure out how to remove this It's only here as a last resort if we + // forget to explicitly flush. immediateQueueCallbackNode = Scheduler_scheduleCallback( Scheduler_ImmediatePriority, flushSyncCallbackQueueImpl, diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 4e42e60ebac94..4c7d308a9c909 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -1810,18 +1810,10 @@ describe('ReactHooksWithNoopRenderer', () => { await act(async () => { ReactNoop.renderLegacySyncRoot(); - if (gate(flags => flags.enableDiscreteEventMicroTasks)) { - // Flush microtasks. - await null; - - // Even in legacy mode, effects are deferred until after paint - expect(Scheduler).toHaveYielded(['Count: (empty)']); - expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); - } else { - // Even in legacy mode, effects are deferred until after paint - expect(Scheduler).toFlushAndYieldThrough(['Count: (empty)']); - expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); - } + // Even in legacy mode, effects are deferred until after paint + ReactNoop.flushSync(); + expect(Scheduler).toHaveYielded(['Count: (empty)']); + expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]); }); // effects get forced on exiting act() diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index f568191681b67..613fe89d63279 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1400,6 +1400,10 @@ describe('ReactIncrementalErrorHandling', () => { 'BrokenRenderAndUnmount componentWillUnmount', ]); expect(ReactNoop.getChildren()).toEqual([]); + + expect(() => { + ReactNoop.flushSync(); + }).toThrow('One does not simply unmount me.'); }); it('does not interrupt unmounting if detaching a ref throws', () => { diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js index bbd55f168ff42..b3f14e6ad1efc 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreen-test.js @@ -98,16 +98,11 @@ describe('ReactOffscreen', () => { , ); - if (gate(flags => flags.enableDiscreteEventMicroTasks)) { - // Flush microtasks. - await null; - // Should not defer the hidden tree - expect(Scheduler).toHaveYielded(['A', 'Outside']); - } else { - // Should not defer the hidden tree - expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']); - } + ReactNoop.flushSync(); + + // Should not defer the hidden tree + expect(Scheduler).toHaveYielded(['A', 'Outside']); }); expect(root).toMatchRenderedOutput( <> diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js index a1ebb3065c957..ab9995d01cb5f 100644 --- a/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.js @@ -569,17 +569,11 @@ describe( ReactNoop.render(); }); - if (gate(flags => flags.enableDiscreteEventMicroTasks)) { - await null; - - // Because the render expired, React should finish the tree without - // consulting `shouldYield` again - expect(Scheduler).toHaveYielded(['B', 'C']); - } else { - // Because the render expired, React should finish the tree without - // consulting `shouldYield` again - expect(Scheduler).toFlushExpired(['B', 'C']); - } + ReactNoop.flushSync(); + + // Because the render expired, React should finish the tree without + // consulting `shouldYield` again + expect(Scheduler).toHaveYielded(['B', 'C']); }); }); }, diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index 15f7eba92e51b..b6e34104ad61f 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -331,14 +331,10 @@ describe('ReactSuspensePlaceholder', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); - if (gate(flags => flags.enableDiscreteEventMicroTasks)) { - // Flush microtasks - await null; - - expect(Scheduler).toHaveYielded(['Loaded']); - } else { - expect(Scheduler).toFlushExpired(['Loaded']); - } + + ReactNoop.flushSync(); + + expect(Scheduler).toHaveYielded(['Loaded']); expect(ReactNoop).toMatchRenderedOutput('LoadedText'); expect(onRender).toHaveBeenCalledTimes(2); @@ -435,14 +431,9 @@ describe('ReactSuspensePlaceholder', () => { expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']); - if (gate(flags => flags.enableDiscreteEventMicroTasks)) { - // Flush microtasks - await null; + ReactNoop.flushSync(); - expect(Scheduler).toHaveYielded(['Loaded']); - } else { - expect(Scheduler).toFlushExpired(['Loaded']); - } + expect(Scheduler).toHaveYielded(['Loaded']); expect(ReactNoop).toMatchRenderedOutput('LoadedNew'); expect(onRender).toHaveBeenCalledTimes(4); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 3d0362f4b3fcd..56ad7fa587afd 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -151,4 +151,6 @@ export const enableNonInterruptingNormalPri = false; export const enableDiscreteEventMicroTasks = false; +export const enableSyncMicroTasks = false; + export const enableNativeEventPriorityInference = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index f97f4f428c181..790887b7e7cee 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -59,6 +59,7 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableNonInterruptingNormalPri = false; export const enableDiscreteEventMicroTasks = false; +export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = 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 9d160790b3fec..8b54ff76d2f6e 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -58,6 +58,7 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableNonInterruptingNormalPri = false; export const enableDiscreteEventMicroTasks = false; +export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = 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 52990e5ec7b71..78f1ed1b9a098 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -58,6 +58,7 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableNonInterruptingNormalPri = false; export const enableDiscreteEventMicroTasks = false; +export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = 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 3c739b7bccb39..c25b8e6e04029 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -58,6 +58,7 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableNonInterruptingNormalPri = false; export const enableDiscreteEventMicroTasks = false; +export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = 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 fb2211763c03d..ff5c7c43d0864 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -58,6 +58,7 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableNonInterruptingNormalPri = false; export const enableDiscreteEventMicroTasks = false; +export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = 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 34e6d5315742f..696de3ce78dd7 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -58,6 +58,7 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableNonInterruptingNormalPri = false; export const enableDiscreteEventMicroTasks = false; +export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = 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 4f0c56733eafd..d33d0683c15a7 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -58,6 +58,7 @@ export const enableRecursiveCommitTraversal = false; export const disableSchedulerTimeoutInWorkLoop = false; export const enableNonInterruptingNormalPri = false; export const enableDiscreteEventMicroTasks = false; +export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = 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 b6e1d4260cc4a..b43f53fbfae19 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -57,4 +57,5 @@ export const enableProfilerNestedUpdateScheduledHook = __VARIANT__; export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; export const enableNonInterruptingNormalPri = __VARIANT__; export const enableDiscreteEventMicroTasks = __VARIANT__; +export const enableSyncMicroTasks = __VARIANT__; export const enableNativeEventPriorityInference = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 924fd6ea9e5bd..21cc7979df98c 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -33,6 +33,7 @@ export const { disableSchedulerTimeoutInWorkLoop, enableNonInterruptingNormalPri, enableDiscreteEventMicroTasks, + enableSyncMicroTasks, enableNativeEventPriorityInference, } = dynamicFeatureFlags; From 18555c320a3f6886bfad42823576e278adcf29d1 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Thu, 25 Feb 2021 15:16:07 -0700 Subject: [PATCH 3/3] Fix comment --- .../src/SchedulerWithReactIntegration.new.js | 7 ++++--- .../src/SchedulerWithReactIntegration.old.js | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js index 8d4f4c553a227..1c13e3a8fda90 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.new.js @@ -146,13 +146,14 @@ export function scheduleSyncCallback(callback: SchedulerCallback) { // the next tick, or earlier if something calls `flushSyncCallbackQueue`. if (syncQueue === null) { syncQueue = [callback]; + + // TODO: Figure out how to remove this It's only here as a last resort if we + // forget to explicitly flush. if (enableSyncMicroTasks && supportsMicrotasks) { // Flush the queue in a microtask. scheduleMicrotask(flushSyncCallbackQueueImpl); } else { - // Flush the queue in the next tick, at the earliest. - // TODO: Figure out how to remove this It's only here as a last resort if we - // forget to explicitly flush. + // Flush the queue in the next tick. immediateQueueCallbackNode = Scheduler_scheduleCallback( Scheduler_ImmediatePriority, flushSyncCallbackQueueImpl, diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js index 96a1077b380df..5c4451499ca5b 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.old.js @@ -146,13 +146,14 @@ export function scheduleSyncCallback(callback: SchedulerCallback) { // the next tick, or earlier if something calls `flushSyncCallbackQueue`. if (syncQueue === null) { syncQueue = [callback]; + + // TODO: Figure out how to remove this It's only here as a last resort if we + // forget to explicitly flush. if (enableSyncMicroTasks && supportsMicrotasks) { // Flush the queue in a microtask. scheduleMicrotask(flushSyncCallbackQueueImpl); } else { - // Flush the queue in the next tick, at the earliest. - // TODO: Figure out how to remove this It's only here as a last resort if we - // forget to explicitly flush. + // Flush the queue in the next tick. immediateQueueCallbackNode = Scheduler_scheduleCallback( Scheduler_ImmediatePriority, flushSyncCallbackQueueImpl,