Schedule sync updates in microtask (#20872)

* Schedule sync updates in microtask * Updates from review * Fix comment
facebook · Feb 25, 2021 · c581cdd · c581cdd
1 parent 90bde65
commit c581cdd
Show file tree

Hide file tree

Showing 23 changed files with 121 additions and 51 deletions.
diff --git 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
@@ -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
@@ -16,13 +16,15 @@ import {__interactionsRef} from 'scheduler/tracing';
 import {
  enableSchedulerTracing,
  decoupleUpdatePriorityFromScheduler,
+ enableSyncMicroTasks,
 } from 'shared/ReactFeatureFlags';
 import invariant from 'shared/invariant';
 import {
  SyncLanePriority,
  getCurrentUpdateLanePriority,
  setCurrentUpdateLanePriority,
 } from './ReactFiberLane.new';
+import {scheduleMicrotask, supportsMicrotasks} from './ReactFiberHostConfig';
 
 const {
  unstable_runWithPriority: Scheduler_runWithPriority,
@@ -144,13 +146,19 @@ 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.
- immediateQueueCallbackNode = Scheduler_scheduleCallback(
- Scheduler_ImmediatePriority,
- flushSyncCallbackQueueImpl,
- );
+ if (enableSyncMicroTasks && supportsMicrotasks) {
+ // Flush the queue in a microtask.
+ scheduleMicrotask(flushSyncCallbackQueueImpl);
+ } else {
+ // Flush the queue in the next tick.
+ 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
@@ -16,13 +16,15 @@ import {__interactionsRef} from 'scheduler/tracing';
 import {
  enableSchedulerTracing,
  decoupleUpdatePriorityFromScheduler,
+ enableSyncMicroTasks,
 } from 'shared/ReactFeatureFlags';
 import invariant from 'shared/invariant';
 import {
  SyncLanePriority,
  getCurrentUpdateLanePriority,
  setCurrentUpdateLanePriority,
 } from './ReactFiberLane.old';
+import {scheduleMicrotask, supportsMicrotasks} from './ReactFiberHostConfig';
 
 const {
  unstable_runWithPriority: Scheduler_runWithPriority,
@@ -144,13 +146,19 @@ 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.
- immediateQueueCallbackNode = Scheduler_scheduleCallback(
- Scheduler_ImmediatePriority,
- flushSyncCallbackQueueImpl,
- );
+ if (enableSyncMicroTasks && supportsMicrotasks) {
+ // Flush the queue in a microtask.
+ scheduleMicrotask(flushSyncCallbackQueueImpl);
+ } else {
+ // Flush the queue in the next tick.
+ 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
@@ -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
@@ -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,10 +1807,12 @@ describe('ReactHooksWithNoopRenderer', () => {
  }, [props.count]);
  return <Text text={'Count: ' + count} />;
  }
- act(() => {
+ await act(async () => {
  ReactNoop.renderLegacySyncRoot(<Counter count={0} />);
+
  // Even in legacy mode, effects are deferred until after paint
- expect(Scheduler).toFlushAndYieldThrough(['Count: (empty)']);
+ ReactNoop.flushSync();
+ expect(Scheduler).toHaveYielded(['Count: (empty)']);
  expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
  });
 

diff --git 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
@@ -98,8 +98,11 @@ describe('ReactOffscreen', () => {
  <Text text="Outside" />
  </>,
  );
+
+ ReactNoop.flushSync();
+
  // Should not defer the hidden tree
- expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']);
+ 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
@@ -569,9 +569,11 @@ describe(
  ReactNoop.render(<App />);
  });
 
+ ReactNoop.flushSync();
+
  // Because the render expired, React should finish the tree without
  // consulting `shouldYield` again
- expect(Scheduler).toFlushExpired(['B', 'C']);
+ expect(Scheduler).toHaveYielded(['B', 'C']);
  });
  });
  },

diff --git 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
@@ -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(<App shouldSuspend={true} />);
  expect(Scheduler).toHaveYielded([
  'App',
@@ -331,7 +331,10 @@ describe('ReactSuspensePlaceholder', () => {
  jest.advanceTimersByTime(1000);
 
  expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']);
- expect(Scheduler).toFlushExpired(['Loaded']);
+
+ ReactNoop.flushSync();
+
+ expect(Scheduler).toHaveYielded(['Loaded']);
  expect(ReactNoop).toMatchRenderedOutput('LoadedText');
  expect(onRender).toHaveBeenCalledTimes(2);
 
@@ -378,7 +381,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(
  <App shouldSuspend={false} textRenderDuration={5} />,
  );
@@ -427,7 +430,10 @@ describe('ReactSuspensePlaceholder', () => {
  jest.advanceTimersByTime(1000);
 
  expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']);
- expect(Scheduler).toFlushExpired(['Loaded']);
+
+ ReactNoop.flushSync();
+
+ expect(Scheduler).toHaveYielded(['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
@@ -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(
  <>
  <span prop="Step: 2" />
@@ -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(
  <>
  <span prop="A" />
@@ -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]',