@samitouri / QOS-React-2 / commits / be5a2e231a

Land enableSyncMicrotasks (#20979)

Andrew Clark committed Mar 19, 2021 at 17:28 UTC be5a2e231ae27b2c64806c026a5b9921be5e7560
18 files changed +100 -60
packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js
+10 -8
@@ -243,11 +243,12 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
243 mouseOverEvent.initEvent('mouseover', true, true);
244 dispatchAndSetCurrentEvent(target.current, mouseOverEvent);
245
246 - // 3s should be enough to expire the updates
247 - Scheduler.unstable_advanceTime(3000);
248 - expect(Scheduler).toFlushExpired([]);
249 - expect(container.textContent).toEqual('hovered');
246 + // Flush discrete updates
247 + ReactDOM.flushSync();
248 + // Since mouse over is not discrete, should not have updated yet
249 + expect(container.textContent).toEqual('not hovered');
250 });
251 + expect(container.textContent).toEqual('hovered');
252 });
253
254 // @gate experimental
@@ -275,11 +276,12 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
276 mouseEnterEvent.initEvent('mouseenter', true, true);
277 dispatchAndSetCurrentEvent(target.current, mouseEnterEvent);
278
278 - // 3s should be enough to expire the updates
279 - Scheduler.unstable_advanceTime(3000);
280 - expect(Scheduler).toFlushExpired([]);
281 - expect(container.textContent).toEqual('hovered');
279 + // Flush discrete updates
280 + ReactDOM.flushSync();
281 + // Since mouse end is not discrete, should not have updated yet
282 + expect(container.textContent).toEqual('not hovered');
283 });
284 + expect(container.textContent).toEqual('hovered');
285 });
286
287 // @gate experimental
packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js
+5 -4
@@ -761,11 +761,12 @@ describe('ChangeEventPlugin', () => {
761 mouseOverEvent.initEvent('mouseover', true, true);
762 target.current.dispatchEvent(mouseOverEvent);
763
764 - // 3s should be enough to expire the updates
765 - Scheduler.unstable_advanceTime(3000);
766 - expect(Scheduler).toFlushExpired([]);
767 - expect(container.textContent).toEqual('hovered');
764 + // Flush discrete updates
765 + ReactDOM.flushSync();
766 + // Since mouse enter/leave is not discrete, should not have updated yet
767 + expect(container.textContent).toEqual('not hovered');
768 });
769 + expect(container.textContent).toEqual('hovered');
770 });
771 });
772 });
packages/react-reconciler/src/SchedulerWithReactIntegration.new.js
+2 -5
@@ -13,10 +13,7 @@ import type {ReactPriorityLevel} from './ReactInternalTypes';
13 // CommonJS interop named imports.
14 import * as Scheduler from 'scheduler';
15 import {__interactionsRef} from 'scheduler/tracing';
16 -import {
17 - enableSchedulerTracing,
18 - enableSyncMicroTasks,
19 -} from 'shared/ReactFeatureFlags';
16 +import {enableSchedulerTracing} from 'shared/ReactFeatureFlags';
17 import invariant from 'shared/invariant';
18 import {
19 SyncLanePriority,
@@ -139,7 +136,7 @@ export function scheduleSyncCallback(callback: SchedulerCallback) {
136
137 // TODO: Figure out how to remove this It's only here as a last resort if we
138 // forget to explicitly flush.
142 - if (enableSyncMicroTasks && supportsMicrotasks) {
139 + if (supportsMicrotasks) {
140 // Flush the queue in a microtask.
141 scheduleMicrotask(flushSyncCallbackQueueImpl);
142 } else {
packages/react-reconciler/src/SchedulerWithReactIntegration.old.js
+2 -5
@@ -13,10 +13,7 @@ import type {ReactPriorityLevel} from './ReactInternalTypes';
13 // CommonJS interop named imports.
14 import * as Scheduler from 'scheduler';
15 import {__interactionsRef} from 'scheduler/tracing';
16 -import {
17 - enableSchedulerTracing,
18 - enableSyncMicroTasks,
19 -} from 'shared/ReactFeatureFlags';
16 +import {enableSchedulerTracing} from 'shared/ReactFeatureFlags';
17 import invariant from 'shared/invariant';
18 import {
19 SyncLanePriority,
@@ -139,7 +136,7 @@ export function scheduleSyncCallback(callback: SchedulerCallback) {
136
137 // TODO: Figure out how to remove this It's only here as a last resort if we
138 // forget to explicitly flush.
142 - if (enableSyncMicroTasks && supportsMicrotasks) {
139 + if (supportsMicrotasks) {
140 // Flush the queue in a microtask.
141 scheduleMicrotask(flushSyncCallbackQueueImpl);
142 } else {
packages/react-reconciler/src/__tests__/ReactExpiration-test.js
+25 -10
@@ -103,23 +103,32 @@ describe('ReactExpiration', () => {
103 return {type: 'span', children: [], prop, hidden: false};
104 }
105
106 + function flushNextRenderIfExpired() {
107 + // This will start rendering the next level of work. If the work hasn't
108 + // expired yet, React will exit without doing anything. If it has expired,
109 + // it will schedule a sync task.
110 + Scheduler.unstable_flushExpired();
111 + // Flush the sync task.
112 + ReactNoop.flushSync();
113 + }
114 +
115 it('increases priority of updates as time progresses', () => {
116 ReactNoop.render(<span prop="done" />);
117
118 expect(ReactNoop.getChildren()).toEqual([]);
119
120 // Nothing has expired yet because time hasn't advanced.
112 - ReactNoop.flushExpired();
121 + flushNextRenderIfExpired();
122 expect(ReactNoop.getChildren()).toEqual([]);
123
124 // Advance time a bit, but not enough to expire the low pri update.
125 ReactNoop.expire(4500);
117 - ReactNoop.flushExpired();
126 + flushNextRenderIfExpired();
127 expect(ReactNoop.getChildren()).toEqual([]);
128
129 // Advance by another second. Now the update should expire and flush.
121 - ReactNoop.expire(1000);
122 - ReactNoop.flushExpired();
130 + ReactNoop.expire(500);
131 + flushNextRenderIfExpired();
132 expect(ReactNoop.getChildren()).toEqual([span('done')]);
133 });
134
@@ -323,7 +332,8 @@ describe('ReactExpiration', () => {
332
333 Scheduler.unstable_advanceTime(10000);
334
326 - expect(Scheduler).toFlushExpired(['D', 'E']);
335 + flushNextRenderIfExpired();
336 + expect(Scheduler).toHaveYielded(['D', 'E']);
337 expect(root).toMatchRenderedOutput('ABCDE');
338 });
339
@@ -351,7 +361,8 @@ describe('ReactExpiration', () => {
361
362 Scheduler.unstable_advanceTime(10000);
363
354 - expect(Scheduler).toFlushExpired(['D', 'E']);
364 + flushNextRenderIfExpired();
365 + expect(Scheduler).toHaveYielded(['D', 'E']);
366 expect(root).toMatchRenderedOutput('ABCDE');
367 });
368
@@ -373,12 +384,14 @@ describe('ReactExpiration', () => {
384 ReactNoop.render('Hi');
385
386 // The update should not have expired yet.
376 - expect(Scheduler).toFlushExpired([]);
387 + flushNextRenderIfExpired();
388 + expect(Scheduler).toHaveYielded([]);
389 expect(ReactNoop).toMatchRenderedOutput(null);
390
391 // Advance the time some more to expire the update.
392 Scheduler.unstable_advanceTime(10000);
381 - expect(Scheduler).toFlushExpired([]);
393 + flushNextRenderIfExpired();
394 + expect(Scheduler).toHaveYielded([]);
395 expect(ReactNoop).toMatchRenderedOutput('Hi');
396 });
397
@@ -391,7 +404,8 @@ describe('ReactExpiration', () => {
404 Scheduler.unstable_advanceTime(10000);
405
406 ReactNoop.render('Hi');
394 - expect(Scheduler).toFlushExpired([]);
407 + flushNextRenderIfExpired();
408 + expect(Scheduler).toHaveYielded([]);
409 expect(ReactNoop).toMatchRenderedOutput(null);
410
411 // Advancing by ~5 seconds should be sufficient to expire the update. (I
@@ -399,7 +413,8 @@ describe('ReactExpiration', () => {
413 Scheduler.unstable_advanceTime(6000);
414
415 ReactNoop.render('Hi');
402 - expect(Scheduler).toFlushExpired([]);
416 + flushNextRenderIfExpired();
417 + expect(Scheduler).toHaveYielded([]);
418 expect(ReactNoop).toMatchRenderedOutput('Hi');
419 });
420
packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
+6 -2
@@ -402,7 +402,7 @@ describe('ReactIncrementalErrorHandling', () => {
402 expect(ReactNoop.getChildren()).toEqual([]);
403 });
404
405 - it('retries one more time if an error occurs during a render that expires midway through the tree', () => {
405 + it('retries one more time if an error occurs during a render that expires midway through the tree', async () => {
406 function Oops({unused}) {
407 Scheduler.unstable_yieldValue('Oops');
408 throw new Error('Oops');
@@ -432,7 +432,11 @@ describe('ReactIncrementalErrorHandling', () => {
432
433 // Expire the render midway through
434 Scheduler.unstable_advanceTime(10000);
435 - expect(() => Scheduler.unstable_flushExpired()).toThrow('Oops');
435 +
436 + expect(() => {
437 + Scheduler.unstable_flushExpired();
438 + ReactNoop.flushSync();
439 + }).toThrow('Oops');
440
441 expect(Scheduler).toHaveYielded([
442 // The render expired, but we shouldn't throw out the partial work.
packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
+19 -5
@@ -32,6 +32,15 @@ describe('ReactIncrementalUpdates', () => {
32 return {type: 'span', children: [], prop, hidden: false};
33 }
34
35 + function flushNextRenderIfExpired() {
36 + // This will start rendering the next level of work. If the work hasn't
37 + // expired yet, React will exit without doing anything. If it has expired,
38 + // it will schedule a sync task.
39 + Scheduler.unstable_flushExpired();
40 + // Flush the sync task.
41 + ReactNoop.flushSync();
42 + }
43 +
44 it('applies updates in order of priority', () => {
45 let state;
46 class Foo extends React.Component {
@@ -469,7 +478,8 @@ describe('ReactIncrementalUpdates', () => {
478
479 ReactNoop.act(() => {
480 ReactNoop.render(<App />);
472 - expect(Scheduler).toFlushExpired([]);
481 + flushNextRenderIfExpired();
482 + expect(Scheduler).toHaveYielded([]);
483 expect(Scheduler).toFlushAndYield([
484 'Render: 0',
485 'Commit: 0',
@@ -479,7 +489,8 @@ describe('ReactIncrementalUpdates', () => {
489 Scheduler.unstable_advanceTime(10000);
490
491 setCount(2);
482 - expect(Scheduler).toFlushExpired([]);
492 + flushNextRenderIfExpired();
493 + expect(Scheduler).toHaveYielded([]);
494 });
495 });
496
@@ -497,7 +508,8 @@ describe('ReactIncrementalUpdates', () => {
508 Scheduler.unstable_advanceTime(10000);
509
510 ReactNoop.render(<Text text="B" />);
500 - expect(Scheduler).toFlushExpired([]);
511 + flushNextRenderIfExpired();
512 + expect(Scheduler).toHaveYielded([]);
513 });
514
515 it('regression: does not expire soon due to previous expired work', () => {
@@ -508,12 +520,14 @@ describe('ReactIncrementalUpdates', () => {
520
521 ReactNoop.render(<Text text="A" />);
522 Scheduler.unstable_advanceTime(10000);
511 - expect(Scheduler).toFlushExpired(['A']);
523 + flushNextRenderIfExpired();
524 + expect(Scheduler).toHaveYielded(['A']);
525
526 Scheduler.unstable_advanceTime(10000);
527
528 ReactNoop.render(<Text text="B" />);
516 - expect(Scheduler).toFlushExpired([]);
529 + flushNextRenderIfExpired();
530 + expect(Scheduler).toHaveYielded([]);
531 });
532
533 it('when rebasing, does not exclude updates that were already committed, regardless of priority', async () => {
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
+31 -10
@@ -984,6 +984,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
984 expect(ReactNoop.getChildren()).toEqual([span('C')]);
985 });
986
987 + // TODO: This test was written against the old Expiration Times
988 + // implementation. It doesn't really test what it was intended to test
989 + // anymore, because all updates to the same queue get entangled together.
990 + // Even if they haven't expired. Consider either deleting or rewriting.
991 // @gate enableCache
992 it('flushes all expired updates in a single batch', async () => {
993 class Foo extends React.Component {
@@ -1013,10 +1017,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1017 jest.advanceTimersByTime(1000);
1018 ReactNoop.render(<Foo text="goodbye" />);
1019
1016 - Scheduler.unstable_advanceTime(10000);
1017 - jest.advanceTimersByTime(10000);
1018 -
1019 - expect(Scheduler).toFlushExpired([
1020 + expect(Scheduler).toFlushAndYield([
1021 'Suspend! [goodbye]',
1022 'Loading...',
1023 'Commit: goodbye',
@@ -1797,12 +1798,32 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1798 await advanceTimers(5000);
1799
1800 // Retry with the new content.
1800 - expect(Scheduler).toFlushAndYield([
1801 - 'A',
1802 - // B still suspends
1803 - 'Suspend! [B]',
1804 - 'Loading more...',
1805 - ]);
1801 + if (gate(flags => flags.disableSchedulerTimeoutInWorkLoop)) {
1802 + expect(Scheduler).toFlushAndYield([
1803 + 'A',
1804 + // B still suspends
1805 + 'Suspend! [B]',
1806 + 'Loading more...',
1807 + ]);
1808 + } else {
1809 + // In this branch, right as we start rendering, we detect that the work
1810 + // has expired (via Scheduler's didTimeout argument) and re-schedule the
1811 + // work as synchronous. Since sync work does not flow through Scheduler,
1812 + // we need to use `flushSync`.
1813 + //
1814 + // Usually we would use `act`, which fluses both sync work and Scheduler
1815 + // work, but that would also force the fallback to display, and this test
1816 + // is specifically about whether we delay or show the fallback.
1817 + expect(Scheduler).toFlushAndYield([]);
1818 + // This will flush the synchronous callback we just scheduled.
1819 + ReactNoop.flushSync();
1820 + expect(Scheduler).toHaveYielded([
1821 + 'A',
1822 + // B still suspends
1823 + 'Suspend! [B]',
1824 + 'Loading more...',
1825 + ]);
1826 + }
1827 // Because we've already been waiting for so long we've exceeded
1828 // our threshold and we show the next level immediately.
1829 expect(ReactNoop.getChildren()).toEqual([
packages/shared/ReactFeatureFlags.js
-2
@@ -150,6 +150,4 @@ export const enableRecursiveCommitTraversal = false;
150
151 export const disableSchedulerTimeoutInWorkLoop = false;
152
153 -export const enableSyncMicroTasks = false;
154 -
153 export const enableLazyContextPropagation = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
-1
@@ -57,7 +57,6 @@ export const enableUseRefAccessWarning = false;
57
58 export const enableRecursiveCommitTraversal = false;
59 export const disableSchedulerTimeoutInWorkLoop = false;
60 -export const enableSyncMicroTasks = false;
60 export const enableLazyContextPropagation = false;
61
62 // Flow magic to verify the exports of this file match the original version.
packages/shared/forks/ReactFeatureFlags.native-oss.js
-1
@@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false;
56
57 export const enableRecursiveCommitTraversal = false;
58 export const disableSchedulerTimeoutInWorkLoop = false;
59 -export const enableSyncMicroTasks = false;
59 export const enableLazyContextPropagation = false;
60
61 // Flow magic to verify the exports of this file match the original version.
packages/shared/forks/ReactFeatureFlags.test-renderer.js
-1
@@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false;
56
57 export const enableRecursiveCommitTraversal = false;
58 export const disableSchedulerTimeoutInWorkLoop = false;
59 -export const enableSyncMicroTasks = false;
59 export const enableLazyContextPropagation = false;
60
61 // Flow magic to verify the exports of this file match the original version.
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
-1
@@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false;
56
57 export const enableRecursiveCommitTraversal = false;
58 export const disableSchedulerTimeoutInWorkLoop = false;
59 -export const enableSyncMicroTasks = false;
59 export const enableLazyContextPropagation = false;
60
61 // Flow magic to verify the exports of this file match the original version.
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
-1
@@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false;
56
57 export const enableRecursiveCommitTraversal = false;
58 export const disableSchedulerTimeoutInWorkLoop = false;
59 -export const enableSyncMicroTasks = false;
59 export const enableLazyContextPropagation = false;
60
61 // Flow magic to verify the exports of this file match the original version.
packages/shared/forks/ReactFeatureFlags.testing.js
-1
@@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false;
56
57 export const enableRecursiveCommitTraversal = false;
58 export const disableSchedulerTimeoutInWorkLoop = false;
59 -export const enableSyncMicroTasks = false;
59 export const enableLazyContextPropagation = false;
60
61 // Flow magic to verify the exports of this file match the original version.
packages/shared/forks/ReactFeatureFlags.testing.www.js
-1
@@ -56,7 +56,6 @@ export const enableUseRefAccessWarning = false;
56
57 export const enableRecursiveCommitTraversal = false;
58 export const disableSchedulerTimeoutInWorkLoop = false;
59 -export const enableSyncMicroTasks = false;
59 export const enableLazyContextPropagation = false;
60
61 // Flow magic to verify the exports of this file match the original version.
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
-1
@@ -55,5 +55,4 @@ export const enableUseRefAccessWarning = __VARIANT__;
55
56 export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
57 export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
58 -export const enableSyncMicroTasks = __VARIANT__;
58 export const enableLazyContextPropagation = __VARIANT__;
packages/shared/forks/ReactFeatureFlags.www.js
-1
@@ -31,7 +31,6 @@ export const {
31 enableUseRefAccessWarning,
32 disableNativeComponentFrames,
33 disableSchedulerTimeoutInWorkLoop,
34 - enableSyncMicroTasks,
34 enableLazyContextPropagation,
35 } = dynamicFeatureFlags;
36