Skip to content

Commit c581cdd

Browse files
rickhanloniirickhanlonii
authored
Schedule sync updates in microtask (#20872)
* Schedule sync updates in microtask * Updates from review * Fix comment
1 parent 90bde65 commit c581cdd

23 files changed

+121
-51
lines changed

‎packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,7 @@ describe('ReactDOMServerPartialHydration', () => {
380380
resolve();
381381
await promise;
382382
Scheduler.unstable_flushAll();
383+
await null;
383384
jest.runAllTimers();
384385

385386
// We should now have hydrated with a ref on the existing span.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@ describe('ReactDOMServerHydration', () => {
488488
jest.runAllTimers();
489489
await Promise.resolve();
490490
Scheduler.unstable_flushAll();
491+
await null;
491492
expect(element.textContent).toBe('Hello world');
492493
});
493494

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ import {__interactionsRef} from 'scheduler/tracing';
1616
import {
1717
enableSchedulerTracing,
1818
decoupleUpdatePriorityFromScheduler,
19+
enableSyncMicroTasks,
1920
} from 'shared/ReactFeatureFlags';
2021
import invariant from 'shared/invariant';
2122
import {
2223
SyncLanePriority,
2324
getCurrentUpdateLanePriority,
2425
setCurrentUpdateLanePriority,
2526
} from './ReactFiberLane.new';
27+
import {scheduleMicrotask, supportsMicrotasks} from './ReactFiberHostConfig';
2628

2729
const {
2830
unstable_runWithPriority: Scheduler_runWithPriority,
@@ -144,13 +146,19 @@ export function scheduleSyncCallback(callback: SchedulerCallback) {
144146
// the next tick, or earlier if something calls `flushSyncCallbackQueue`.
145147
if (syncQueue === null) {
146148
syncQueue = [callback];
147-
// Flush the queue in the next tick, at the earliest.
149+
148150
// TODO: Figure out how to remove this It's only here as a last resort if we
149151
// forget to explicitly flush.
150-
immediateQueueCallbackNode = Scheduler_scheduleCallback(
151-
Scheduler_ImmediatePriority,
152-
flushSyncCallbackQueueImpl,
153-
);
152+
if (enableSyncMicroTasks && supportsMicrotasks) {
153+
// Flush the queue in a microtask.
154+
scheduleMicrotask(flushSyncCallbackQueueImpl);
155+
} else {
156+
// Flush the queue in the next tick.
157+
immediateQueueCallbackNode = Scheduler_scheduleCallback(
158+
Scheduler_ImmediatePriority,
159+
flushSyncCallbackQueueImpl,
160+
);
161+
}
154162
} else {
155163
// Push onto existing queue. Don't need to schedule a callback because
156164
// we already scheduled one when we created the queue.

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ import {__interactionsRef} from 'scheduler/tracing';
1616
import {
1717
enableSchedulerTracing,
1818
decoupleUpdatePriorityFromScheduler,
19+
enableSyncMicroTasks,
1920
} from 'shared/ReactFeatureFlags';
2021
import invariant from 'shared/invariant';
2122
import {
2223
SyncLanePriority,
2324
getCurrentUpdateLanePriority,
2425
setCurrentUpdateLanePriority,
2526
} from './ReactFiberLane.old';
27+
import {scheduleMicrotask, supportsMicrotasks} from './ReactFiberHostConfig';
2628

2729
const {
2830
unstable_runWithPriority: Scheduler_runWithPriority,
@@ -144,13 +146,19 @@ export function scheduleSyncCallback(callback: SchedulerCallback) {
144146
// the next tick, or earlier if something calls `flushSyncCallbackQueue`.
145147
if (syncQueue === null) {
146148
syncQueue = [callback];
147-
// Flush the queue in the next tick, at the earliest.
149+
148150
// TODO: Figure out how to remove this It's only here as a last resort if we
149151
// forget to explicitly flush.
150-
immediateQueueCallbackNode = Scheduler_scheduleCallback(
151-
Scheduler_ImmediatePriority,
152-
flushSyncCallbackQueueImpl,
153-
);
152+
if (enableSyncMicroTasks && supportsMicrotasks) {
153+
// Flush the queue in a microtask.
154+
scheduleMicrotask(flushSyncCallbackQueueImpl);
155+
} else {
156+
// Flush the queue in the next tick.
157+
immediateQueueCallbackNode = Scheduler_scheduleCallback(
158+
Scheduler_ImmediatePriority,
159+
flushSyncCallbackQueueImpl,
160+
);
161+
}
154162
} else {
155163
// Push onto existing queue. Don't need to schedule a callback because
156164
// we already scheduled one when we created the queue.

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -598,12 +598,13 @@ describe('ReactExpiration', () => {
598598
// second one.
599599
Scheduler.unstable_advanceTime(1000);
600600
// Attempt to interrupt with a high pri update.
601-
updateHighPri();
601+
await ReactNoop.act(async () => {
602+
updateHighPri();
603+
});
602604

603-
// The first update expired, so first will finish it without
604-
// interrupting. But not the second update, which hasn't expired yet.
605-
expect(Scheduler).toFlushExpired(['Sibling']);
606-
expect(Scheduler).toFlushAndYield([
605+
expect(Scheduler).toHaveYielded([
606+
// The first update expired
607+
'Sibling',
607608
// Then render the high pri update
608609
'High pri: 1',
609610
'Normal pri: 1',

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,7 +1792,7 @@ describe('ReactHooksWithNoopRenderer', () => {
17921792
it(
17931793
'in legacy mode, useEffect is deferred and updates finish synchronously ' +
17941794
'(in a single batch)',
1795-
() => {
1795+
async () => {
17961796
function Counter(props) {
17971797
const [count, updateCount] = useState('(empty)');
17981798
useEffect(() => {
@@ -1807,10 +1807,12 @@ describe('ReactHooksWithNoopRenderer', () => {
18071807
}, [props.count]);
18081808
return <Text text={'Count: ' + count} />;
18091809
}
1810-
act(() => {
1810+
await act(async () => {
18111811
ReactNoop.renderLegacySyncRoot(<Counter count={0} />);
1812+
18121813
// Even in legacy mode, effects are deferred until after paint
1813-
expect(Scheduler).toFlushAndYieldThrough(['Count: (empty)']);
1814+
ReactNoop.flushSync();
1815+
expect(Scheduler).toHaveYielded(['Count: (empty)']);
18141816
expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
18151817
});
18161818

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,6 +1400,10 @@ describe('ReactIncrementalErrorHandling', () => {
14001400
'BrokenRenderAndUnmount componentWillUnmount',
14011401
]);
14021402
expect(ReactNoop.getChildren()).toEqual([]);
1403+
1404+
expect(() => {
1405+
ReactNoop.flushSync();
1406+
}).toThrow('One does not simply unmount me.');
14031407
});
14041408

14051409
it('does not interrupt unmounting if detaching a ref throws', () => {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,11 @@ describe('ReactOffscreen', () => {
9898
<Text text="Outside" />
9999
</>,
100100
);
101+
102+
ReactNoop.flushSync();
103+
101104
// Should not defer the hidden tree
102-
expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']);
105+
expect(Scheduler).toHaveYielded(['A', 'Outside']);
103106
});
104107
expect(root).toMatchRenderedOutput(
105108
<>

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,9 +569,11 @@ describe(
569569
ReactNoop.render(<App />);
570570
});
571571

572+
ReactNoop.flushSync();
573+
572574
// Because the render expired, React should finish the tree without
573575
// consulting `shouldYield` again
574-
expect(Scheduler).toFlushExpired(['B', 'C']);
576+
expect(Scheduler).toHaveYielded(['B', 'C']);
575577
});
576578
});
577579
},

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -292,9 +292,11 @@ describe('ReactSuspenseList', () => {
292292
</>,
293293
);
294294

295-
await C.resolve();
295+
await ReactNoop.act(async () => {
296+
C.resolve();
297+
});
296298

297-
expect(Scheduler).toFlushAndYield(['C']);
299+
expect(Scheduler).toHaveYielded(['C']);
298300

299301
expect(ReactNoop).toMatchRenderedOutput(
300302
<>
@@ -304,9 +306,11 @@ describe('ReactSuspenseList', () => {
304306
</>,
305307
);
306308

307-
await B.resolve();
309+
await ReactNoop.act(async () => {
310+
B.resolve();
311+
});
308312

309-
expect(Scheduler).toFlushAndYield(['B']);
313+
expect(Scheduler).toHaveYielded(['B']);
310314

311315
expect(ReactNoop).toMatchRenderedOutput(
312316
<>

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ describe('ReactSuspensePlaceholder', () => {
310310
});
311311

312312
describe('when suspending during mount', () => {
313-
it('properly accounts for base durations when a suspended times out in a legacy tree', () => {
313+
it('properly accounts for base durations when a suspended times out in a legacy tree', async () => {
314314
ReactNoop.renderLegacySyncRoot(<App shouldSuspend={true} />);
315315
expect(Scheduler).toHaveYielded([
316316
'App',
@@ -331,7 +331,10 @@ describe('ReactSuspensePlaceholder', () => {
331331
jest.advanceTimersByTime(1000);
332332

333333
expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']);
334-
expect(Scheduler).toFlushExpired(['Loaded']);
334+
335+
ReactNoop.flushSync();
336+
337+
expect(Scheduler).toHaveYielded(['Loaded']);
335338
expect(ReactNoop).toMatchRenderedOutput('LoadedText');
336339
expect(onRender).toHaveBeenCalledTimes(2);
337340

@@ -378,7 +381,7 @@ describe('ReactSuspensePlaceholder', () => {
378381
});
379382

380383
describe('when suspending during update', () => {
381-
it('properly accounts for base durations when a suspended times out in a legacy tree', () => {
384+
it('properly accounts for base durations when a suspended times out in a legacy tree', async () => {
382385
ReactNoop.renderLegacySyncRoot(
383386
<App shouldSuspend={false} textRenderDuration={5} />,
384387
);
@@ -427,7 +430,10 @@ describe('ReactSuspensePlaceholder', () => {
427430
jest.advanceTimersByTime(1000);
428431

429432
expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']);
430-
expect(Scheduler).toFlushExpired(['Loaded']);
433+
434+
ReactNoop.flushSync();
435+
436+
expect(Scheduler).toHaveYielded(['Loaded']);
431437
expect(ReactNoop).toMatchRenderedOutput('LoadedNew');
432438
expect(onRender).toHaveBeenCalledTimes(4);
433439

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

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,9 +1088,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
10881088
expect(Scheduler).toHaveYielded(['Suspend! [Result]', 'Loading...']);
10891089
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
10901090

1091-
await resolveText('Result');
1091+
await ReactNoop.act(async () => {
1092+
resolveText('Result');
1093+
});
10921094

1093-
expect(Scheduler).toFlushExpired(['Result']);
1095+
expect(Scheduler).toHaveYielded(['Result']);
10941096
expect(ReactNoop.getChildren()).toEqual([span('Result')]);
10951097
});
10961098

@@ -1156,8 +1158,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
11561158
</>,
11571159
);
11581160

1159-
await resolveText('Step: 2');
1160-
expect(Scheduler).toFlushExpired(['Step: 2']);
1161+
await ReactNoop.act(async () => {
1162+
resolveText('Step: 2');
1163+
});
1164+
expect(Scheduler).toHaveYielded(['Step: 2']);
11611165
expect(ReactNoop).toMatchRenderedOutput(
11621166
<>
11631167
<span prop="Step: 2" />
@@ -1227,9 +1231,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
12271231
</>,
12281232
);
12291233

1230-
await resolveText('B');
1234+
await ReactNoop.act(async () => {
1235+
resolveText('B');
1236+
});
12311237

1232-
expect(Scheduler).toFlushExpired(['B']);
1238+
expect(Scheduler).toHaveYielded(['B']);
12331239
expect(ReactNoop).toMatchRenderedOutput(
12341240
<>
12351241
<span prop="A" />
@@ -1271,9 +1277,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
12711277
]);
12721278
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
12731279

1274-
await resolveText('Hi');
1280+
await ReactNoop.act(async () => {
1281+
resolveText('Hi');
1282+
});
12751283

1276-
expect(Scheduler).toFlushExpired([
1284+
expect(Scheduler).toHaveYielded([
12771285
'constructor',
12781286
'Hi',
12791287
'componentDidMount',
@@ -1316,8 +1324,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
13161324
'Loading...',
13171325
]);
13181326
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1319-
await resolveText('Hi');
1320-
expect(Scheduler).toFlushExpired(['Hi']);
1327+
await ReactNoop.act(async () => {
1328+
resolveText('Hi');
1329+
});
1330+
expect(Scheduler).toHaveYielded(['Hi']);
13211331
expect(ReactNoop.getChildren()).toEqual([span('Hi')]);
13221332
});
13231333

@@ -1360,8 +1370,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
13601370
</>,
13611371
]);
13621372

1363-
await resolveText('Hi');
1364-
expect(Scheduler).toFlushExpired(['Hi']);
1373+
await ReactNoop.act(async () => {
1374+
resolveText('Hi');
1375+
});
1376+
expect(Scheduler).toHaveYielded(['Hi']);
13651377
});
13661378
} else {
13671379
// @gate enableCache
@@ -1401,9 +1413,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
14011413
'Child is hidden: true',
14021414
]);
14031415

1404-
await resolveText('Hi');
1416+
await ReactNoop.act(async () => {
1417+
resolveText('Hi');
1418+
});
14051419

1406-
expect(Scheduler).toFlushExpired(['Hi']);
1420+
expect(Scheduler).toHaveYielded(['Hi']);
14071421
});
14081422
}
14091423

@@ -1647,9 +1661,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
16471661
</>,
16481662
);
16491663

1650-
await resolveText('B');
1664+
await ReactNoop.act(async () => {
1665+
resolveText('B');
1666+
});
16511667

1652-
expect(Scheduler).toFlushAndYield([
1668+
expect(Scheduler).toHaveYielded([
16531669
'B',
16541670
'Destroy Layout Effect [Loading...]',
16551671
'Layout Effect [B]',
@@ -1681,9 +1697,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
16811697
'Effect [Loading...]',
16821698
]);
16831699

1684-
await resolveText('B2');
1700+
await ReactNoop.act(async () => {
1701+
resolveText('B2');
1702+
});
16851703

1686-
expect(Scheduler).toFlushAndYield([
1704+
expect(Scheduler).toHaveYielded([
16871705
'B2',
16881706
'Destroy Layout Effect [Loading...]',
16891707
'Destroy Layout Effect [B]',

0 commit comments

Comments
 (0)