Skip to content

Commit 73a68fd

Browse files
committed
Move update processing into microtask
When React receives new input (via `setState`, a Suspense promise resolution, and so on), it needs to ensure there's a rendering task associated with the update. Most of this happens `ensureRootIsScheduled`. If a single event contains multiple updates, we end up running the scheduling code once per update. But this is wasteful because we really only need to run it once, at the end of the event (or in the case of flushSync, at the end of the scope function's execution). So this PR moves the scheduling logic to happen in a microtask instead. In some cases, we will force it run earlier than that, like for `flushSync`, but since updates are batched by default, it will almost always happen in the microtask. Even for discrete updates. In production, this should have no observable behavior difference. In a testing environment that uses `act`, this should also not have a behavior difference because React will push these tasks to an internal `act` queue. However, tests that do not use `act` and do not simulate an actual production environment (like an e2e test) may be affected. For example, before this change, if a test were to call `setState` outside of `act` and then immediately call `jest.runAllTimers()`, the update would be synchronously applied. After this change, that will no longer work because the rendering task (a timer, in this case) isn't scheduled until after the microtask queue has run. I don't expect this to be an issue in practice because most people do not write their tests this way. They either use `act`, or they write e2e-style tests. The biggest exception has been... our own internal test suite. Until recently, many of our tests were written in a way that accidentally relied on the updates being scheduled synchronously. Over the past few weeks, @tyao1 and I have gradually converted the test suite to use a new set of testing helpers that are resilient to this implementation detail. (There are also some old Relay tests that were written in the style of React's internal test suite. Those will need to be fixed, too.) The larger motivation behind this change, aside from a minor performance improvement, is we intend to use this new microtask to perform additional logic that doesn't yet exist. Like inferring the priority of a custom event.
1 parent c4a075c commit 73a68fd

10 files changed

+59
-387
lines changed

packages/react-devtools-shared/src/__tests__/profilingCache-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -963,7 +963,7 @@ describe('ProfilingCache', () => {
963963
2 => 0,
964964
},
965965
"passiveEffectDuration": null,
966-
"priorityLevel": "Immediate",
966+
"priorityLevel": "Normal",
967967
"timestamp": 0,
968968
"updaters": [
969969
{

packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -803,9 +803,9 @@ describe('ReactDOMServerSelectiveHydration', () => {
803803
// boundary. See the next test below.
804804
assertLog([
805805
'D',
806-
'Clicked D',
807806
'B', // Ideally this should be later.
808807
'C',
808+
'Clicked D',
809809
'Hover C',
810810
'A',
811811
]);
@@ -963,10 +963,10 @@ describe('ReactDOMServerSelectiveHydration', () => {
963963
// boundary. See the next test below.
964964
assertLog([
965965
'D',
966-
'Clicked D',
967966
'B', // Ideally this should be later.
968967
'C',
969968
// Capture phase isn't replayed
969+
'Clicked D',
970970
// Mouseout isn't replayed
971971
'Mouse Over C',
972972
'Mouse Enter C',

packages/react-reconciler/src/ReactFiberRoot.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ function FiberRootNode(
6363
this.cancelPendingCommit = null;
6464
this.context = null;
6565
this.pendingContext = null;
66+
this.next = null;
6667
this.callbackNode = null;
6768
this.callbackPriority = NoLane;
6869
this.eventTimes = createLaneMap(NoLanes);

packages/react-reconciler/src/ReactFiberRootScheduler.js

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ import type {FiberRoot} from './ReactInternalTypes';
1111
import type {Lane} from './ReactFiberLane';
1212
import type {PriorityLevel} from 'scheduler/src/SchedulerPriorities';
1313

14-
import {enableDeferRootSchedulingToMicrotask} from 'shared/ReactFeatureFlags';
1514
import {
1615
NoLane,
1716
NoLanes,
17+
SyncLane,
1818
getHighestPriorityLane,
1919
getNextLanes,
2020
includesOnlyNonUrgentLanes,
@@ -111,14 +111,6 @@ export function ensureRootIsScheduled(root: FiberRoot): void {
111111
scheduleImmediateTask(processRootScheduleInMicrotask);
112112
}
113113
}
114-
115-
if (!enableDeferRootSchedulingToMicrotask) {
116-
// While this flag is disabled, we schedule the task immediately instead
117-
// of waiting for the microtask to fire.
118-
// TODO: We need to land enableDeferRootSchedulingToMicrotask ASAP to
119-
// unblock additional features we have planned.
120-
scheduleTaskForRootDuringMicrotask(root, now());
121-
}
122114
}
123115

124116
// TODO: Rename to something else. This isn't a generic callback queue anymore.
@@ -255,7 +247,7 @@ function processRootScheduleInMicrotask() {
255247
} else {
256248
// This root still has work. Keep it in the list.
257249
prev = root;
258-
if (includesSyncLane) {
250+
if (includesSyncLane(nextLanes)) {
259251
mightHavePendingSyncWork = true;
260252
}
261253
}
@@ -275,9 +267,6 @@ function scheduleTaskForRootDuringMicrotask(
275267
// rendering task right before we yield to the main thread. It should never be
276268
// called synchronously.
277269
//
278-
// TODO: Unless enableDeferRootSchedulingToMicrotask is off. We need to land
279-
// that ASAP to unblock additional features we have planned.
280-
//
281270
// This function also never performs React work synchronously; it should
282271
// only schedule work to be performed later, in a separate task or microtask.
283272

@@ -306,30 +295,25 @@ function scheduleTaskForRootDuringMicrotask(
306295
) {
307296
// Fast path: There's nothing to work on.
308297
if (existingCallbackNode !== null) {
309-
if (__DEV__ && existingCallbackNode === fakeActCallbackNode) {
310-
// Special `act` case: check if this is the fake callback node used by
311-
// the `act` implementation.
312-
} else {
313-
Scheduler_cancelCallback(existingCallbackNode);
314-
}
298+
cancelCallback(existingCallbackNode);
315299
}
316300
root.callbackNode = null;
317301
root.callbackPriority = NoLane;
318302
return NoLane;
319303
}
320304

321-
// We use the highest priority lane to represent the priority of the callback.
322-
const existingCallbackPriority = root.callbackPriority;
323-
const newCallbackPriority = getHighestPriorityLane(nextLanes);
324-
325305
// Schedule a new callback in the host environment.
326-
if (includesSyncLane(newCallbackPriority)) {
306+
if (includesSyncLane(nextLanes)) {
327307
// Synchronous work will be flushed at the end of the microtask; we don't
328308
// need to schedule anything extra.
329-
mightHavePendingSyncWork = true;
330-
root.callbackPriority = newCallbackPriority;
309+
root.callbackPriority = SyncLane;
331310
root.callbackNode = null;
311+
return SyncLane;
332312
} else {
313+
// We use the highest priority lane to represent the priority of the callback.
314+
const existingCallbackPriority = root.callbackPriority;
315+
const newCallbackPriority = getHighestPriorityLane(nextLanes);
316+
333317
if (
334318
newCallbackPriority === existingCallbackPriority &&
335319
// Special case related to `act`. If the currently scheduled task is a
@@ -374,9 +358,8 @@ function scheduleTaskForRootDuringMicrotask(
374358

375359
root.callbackPriority = newCallbackPriority;
376360
root.callbackNode = newCallbackNode;
361+
return newCallbackPriority;
377362
}
378-
379-
return newCallbackPriority;
380363
}
381364

382365
export type RenderTaskFn = (didTimeout: boolean) => RenderTaskFn | null;

packages/react-reconciler/src/ReactFiberSyncTaskQueue.js

Lines changed: 0 additions & 118 deletions
This file was deleted.

0 commit comments

Comments
 (0)