Skip to content

Commit b729510

Browse files
acdliteacdlite
committed
Land enableSyncMicrotasks
1 parent f6bc9c8 commit b729510

18 files changed

+100
-60
lines changed

‎packages/react-dom/src/__tests__/ReactDOMNativeEventHeuristic-test.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -243,11 +243,12 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
243243
mouseOverEvent.initEvent('mouseover', true, true);
244244
dispatchAndSetCurrentEvent(target.current, mouseOverEvent);
245245

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');
250250
});
251+
expect(container.textContent).toEqual('hovered');
251252
});
252253

253254
// @gate experimental
@@ -275,11 +276,12 @@ describe('ReactDOMNativeEventHeuristic-test', () => {
275276
mouseEnterEvent.initEvent('mouseenter', true, true);
276277
dispatchAndSetCurrentEvent(target.current, mouseEnterEvent);
277278

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');
282283
});
284+
expect(container.textContent).toEqual('hovered');
283285
});
284286

285287
// @gate experimental

‎packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -761,11 +761,12 @@ describe('ChangeEventPlugin', () => {
761761
mouseOverEvent.initEvent('mouseover', true, true);
762762
target.current.dispatchEvent(mouseOverEvent);
763763

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');
768768
});
769+
expect(container.textContent).toEqual('hovered');
769770
});
770771
});
771772
});

‎packages/react-reconciler/src/SchedulerWithReactIntegration.new.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ import type {ReactPriorityLevel} from './ReactInternalTypes';
1313
// CommonJS interop named imports.
1414
import * as Scheduler from 'scheduler';
1515
import {__interactionsRef} from 'scheduler/tracing';
16-
import {
17-
enableSchedulerTracing,
18-
enableSyncMicroTasks,
19-
} from 'shared/ReactFeatureFlags';
16+
import {enableSchedulerTracing} from 'shared/ReactFeatureFlags';
2017
import invariant from 'shared/invariant';
2118
import {
2219
SyncLanePriority,
@@ -139,7 +136,7 @@ export function scheduleSyncCallback(callback: SchedulerCallback) {
139136

140137
// TODO: Figure out how to remove this It's only here as a last resort if we
141138
// forget to explicitly flush.
142-
if (enableSyncMicroTasks && supportsMicrotasks) {
139+
if (supportsMicrotasks) {
143140
// Flush the queue in a microtask.
144141
scheduleMicrotask(flushSyncCallbackQueueImpl);
145142
} else {

‎packages/react-reconciler/src/SchedulerWithReactIntegration.old.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ import type {ReactPriorityLevel} from './ReactInternalTypes';
1313
// CommonJS interop named imports.
1414
import * as Scheduler from 'scheduler';
1515
import {__interactionsRef} from 'scheduler/tracing';
16-
import {
17-
enableSchedulerTracing,
18-
enableSyncMicroTasks,
19-
} from 'shared/ReactFeatureFlags';
16+
import {enableSchedulerTracing} from 'shared/ReactFeatureFlags';
2017
import invariant from 'shared/invariant';
2118
import {
2219
SyncLanePriority,
@@ -139,7 +136,7 @@ export function scheduleSyncCallback(callback: SchedulerCallback) {
139136

140137
// TODO: Figure out how to remove this It's only here as a last resort if we
141138
// forget to explicitly flush.
142-
if (enableSyncMicroTasks && supportsMicrotasks) {
139+
if (supportsMicrotasks) {
143140
// Flush the queue in a microtask.
144141
scheduleMicrotask(flushSyncCallbackQueueImpl);
145142
} else {

‎packages/react-reconciler/src/__tests__/ReactExpiration-test.js‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,23 +103,32 @@ describe('ReactExpiration', () => {
103103
return {type: 'span', children: [], prop, hidden: false};
104104
}
105105

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+
106115
it('increases priority of updates as time progresses', () => {
107116
ReactNoop.render(<span prop="done" />);
108117

109118
expect(ReactNoop.getChildren()).toEqual([]);
110119

111120
// Nothing has expired yet because time hasn't advanced.
112-
ReactNoop.flushExpired();
121+
flushNextRenderIfExpired();
113122
expect(ReactNoop.getChildren()).toEqual([]);
114123

115124
// Advance time a bit, but not enough to expire the low pri update.
116125
ReactNoop.expire(4500);
117-
ReactNoop.flushExpired();
126+
flushNextRenderIfExpired();
118127
expect(ReactNoop.getChildren()).toEqual([]);
119128

120129
// Advance by another second. Now the update should expire and flush.
121-
ReactNoop.expire(1000);
122-
ReactNoop.flushExpired();
130+
ReactNoop.expire(500);
131+
flushNextRenderIfExpired();
123132
expect(ReactNoop.getChildren()).toEqual([span('done')]);
124133
});
125134

@@ -323,7 +332,8 @@ describe('ReactExpiration', () => {
323332

324333
Scheduler.unstable_advanceTime(10000);
325334

326-
expect(Scheduler).toFlushExpired(['D', 'E']);
335+
flushNextRenderIfExpired();
336+
expect(Scheduler).toHaveYielded(['D', 'E']);
327337
expect(root).toMatchRenderedOutput('ABCDE');
328338
});
329339

@@ -351,7 +361,8 @@ describe('ReactExpiration', () => {
351361

352362
Scheduler.unstable_advanceTime(10000);
353363

354-
expect(Scheduler).toFlushExpired(['D', 'E']);
364+
flushNextRenderIfExpired();
365+
expect(Scheduler).toHaveYielded(['D', 'E']);
355366
expect(root).toMatchRenderedOutput('ABCDE');
356367
});
357368

@@ -373,12 +384,14 @@ describe('ReactExpiration', () => {
373384
ReactNoop.render('Hi');
374385

375386
// The update should not have expired yet.
376-
expect(Scheduler).toFlushExpired([]);
387+
flushNextRenderIfExpired();
388+
expect(Scheduler).toHaveYielded([]);
377389
expect(ReactNoop).toMatchRenderedOutput(null);
378390

379391
// Advance the time some more to expire the update.
380392
Scheduler.unstable_advanceTime(10000);
381-
expect(Scheduler).toFlushExpired([]);
393+
flushNextRenderIfExpired();
394+
expect(Scheduler).toHaveYielded([]);
382395
expect(ReactNoop).toMatchRenderedOutput('Hi');
383396
});
384397

@@ -391,15 +404,17 @@ describe('ReactExpiration', () => {
391404
Scheduler.unstable_advanceTime(10000);
392405

393406
ReactNoop.render('Hi');
394-
expect(Scheduler).toFlushExpired([]);
407+
flushNextRenderIfExpired();
408+
expect(Scheduler).toHaveYielded([]);
395409
expect(ReactNoop).toMatchRenderedOutput(null);
396410

397411
// Advancing by ~5 seconds should be sufficient to expire the update. (I
398412
// used a slightly larger number to allow for possible rounding.)
399413
Scheduler.unstable_advanceTime(6000);
400414

401415
ReactNoop.render('Hi');
402-
expect(Scheduler).toFlushExpired([]);
416+
flushNextRenderIfExpired();
417+
expect(Scheduler).toHaveYielded([]);
403418
expect(ReactNoop).toMatchRenderedOutput('Hi');
404419
});
405420

‎packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ describe('ReactIncrementalErrorHandling', () => {
402402
expect(ReactNoop.getChildren()).toEqual([]);
403403
});
404404

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 () => {
406406
function Oops({unused}) {
407407
Scheduler.unstable_yieldValue('Oops');
408408
throw new Error('Oops');
@@ -432,7 +432,11 @@ describe('ReactIncrementalErrorHandling', () => {
432432

433433
// Expire the render midway through
434434
Scheduler.unstable_advanceTime(10000);
435-
expect(() => Scheduler.unstable_flushExpired()).toThrow('Oops');
435+
436+
expect(() => {
437+
Scheduler.unstable_flushExpired();
438+
ReactNoop.flushSync();
439+
}).toThrow('Oops');
436440

437441
expect(Scheduler).toHaveYielded([
438442
// The render expired, but we shouldn't throw out the partial work.

‎packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ describe('ReactIncrementalUpdates', () => {
3232
return {type: 'span', children: [], prop, hidden: false};
3333
}
3434

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+
3544
it('applies updates in order of priority', () => {
3645
let state;
3746
class Foo extends React.Component {
@@ -469,7 +478,8 @@ describe('ReactIncrementalUpdates', () => {
469478

470479
ReactNoop.act(() => {
471480
ReactNoop.render(<App />);
472-
expect(Scheduler).toFlushExpired([]);
481+
flushNextRenderIfExpired();
482+
expect(Scheduler).toHaveYielded([]);
473483
expect(Scheduler).toFlushAndYield([
474484
'Render: 0',
475485
'Commit: 0',
@@ -479,7 +489,8 @@ describe('ReactIncrementalUpdates', () => {
479489
Scheduler.unstable_advanceTime(10000);
480490

481491
setCount(2);
482-
expect(Scheduler).toFlushExpired([]);
492+
flushNextRenderIfExpired();
493+
expect(Scheduler).toHaveYielded([]);
483494
});
484495
});
485496

@@ -497,7 +508,8 @@ describe('ReactIncrementalUpdates', () => {
497508
Scheduler.unstable_advanceTime(10000);
498509

499510
ReactNoop.render(<Text text="B" />);
500-
expect(Scheduler).toFlushExpired([]);
511+
flushNextRenderIfExpired();
512+
expect(Scheduler).toHaveYielded([]);
501513
});
502514

503515
it('regression: does not expire soon due to previous expired work', () => {
@@ -508,12 +520,14 @@ describe('ReactIncrementalUpdates', () => {
508520

509521
ReactNoop.render(<Text text="A" />);
510522
Scheduler.unstable_advanceTime(10000);
511-
expect(Scheduler).toFlushExpired(['A']);
523+
flushNextRenderIfExpired();
524+
expect(Scheduler).toHaveYielded(['A']);
512525

513526
Scheduler.unstable_advanceTime(10000);
514527

515528
ReactNoop.render(<Text text="B" />);
516-
expect(Scheduler).toFlushExpired([]);
529+
flushNextRenderIfExpired();
530+
expect(Scheduler).toHaveYielded([]);
517531
});
518532

519533
it('when rebasing, does not exclude updates that were already committed, regardless of priority', async () => {

‎packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
984984
expect(ReactNoop.getChildren()).toEqual([span('C')]);
985985
});
986986

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.
987991
// @gate enableCache
988992
it('flushes all expired updates in a single batch', async () => {
989993
class Foo extends React.Component {
@@ -1013,10 +1017,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
10131017
jest.advanceTimersByTime(1000);
10141018
ReactNoop.render(<Foo text="goodbye" />);
10151019

1016-
Scheduler.unstable_advanceTime(10000);
1017-
jest.advanceTimersByTime(10000);
1018-
1019-
expect(Scheduler).toFlushExpired([
1020+
expect(Scheduler).toFlushAndYield([
10201021
'Suspend! [goodbye]',
10211022
'Loading...',
10221023
'Commit: goodbye',
@@ -1797,12 +1798,32 @@ describe('ReactSuspenseWithNoopRenderer', () => {
17971798
await advanceTimers(5000);
17981799

17991800
// 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+
}
18061827
// Because we've already been waiting for so long we've exceeded
18071828
// our threshold and we show the next level immediately.
18081829
expect(ReactNoop.getChildren()).toEqual([

‎packages/shared/ReactFeatureFlags.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,4 @@ export const enableRecursiveCommitTraversal = false;
150150

151151
export const disableSchedulerTimeoutInWorkLoop = false;
152152

153-
export const enableSyncMicroTasks = false;
154-
155153
export const enableLazyContextPropagation = false;

‎packages/shared/forks/ReactFeatureFlags.native-fb.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ export const enableUseRefAccessWarning = false;
5757

5858
export const enableRecursiveCommitTraversal = false;
5959
export const disableSchedulerTimeoutInWorkLoop = false;
60-
export const enableSyncMicroTasks = false;
6160
export const enableLazyContextPropagation = false;
6261

6362
// Flow magic to verify the exports of this file match the original version.

0 commit comments

Comments
 (0)