Skip to content

Commit

Permalink
Move update processing into microtask
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed Mar 29, 2023
1 parent c4a075c commit 9faeced
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 370 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -803,9 +803,9 @@ describe('ReactDOMServerSelectiveHydration', () => {
// boundary. See the next test below.
assertLog([
'D',
'Clicked D',
'B', // Ideally this should be later.
'C',
'Clicked D',
'Hover C',
'A',
]);
Expand Down Expand Up @@ -963,10 +963,10 @@ describe('ReactDOMServerSelectiveHydration', () => {
// boundary. See the next test below.
assertLog([
'D',
'Clicked D',
'B', // Ideally this should be later.
'C',
// Capture phase isn't replayed
'Clicked D',
// Mouseout isn't replayed
'Mouse Over C',
'Mouse Enter C',
Expand Down
1 change: 1 addition & 0 deletions packages/react-reconciler/src/ReactFiberRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ function FiberRootNode(
this.cancelPendingCommit = null;
this.context = null;
this.pendingContext = null;
this.next = null;
this.callbackNode = null;
this.callbackPriority = NoLane;
this.eventTimes = createLaneMap(NoLanes);
Expand Down
12 changes: 0 additions & 12 deletions packages/react-reconciler/src/ReactFiberRootScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {FiberRoot} from './ReactInternalTypes';
import type {Lane} from './ReactFiberLane';
import type {PriorityLevel} from 'scheduler/src/SchedulerPriorities';

import {enableDeferRootSchedulingToMicrotask} from 'shared/ReactFeatureFlags';
import {
NoLane,
NoLanes,
Expand Down Expand Up @@ -111,14 +110,6 @@ export function ensureRootIsScheduled(root: FiberRoot): void {
scheduleImmediateTask(processRootScheduleInMicrotask);
}
}

if (!enableDeferRootSchedulingToMicrotask) {
// While this flag is disabled, we schedule the task immediately instead
// of waiting for the microtask to fire.
// TODO: We need to land enableDeferRootSchedulingToMicrotask ASAP to
// unblock additional features we have planned.
scheduleTaskForRootDuringMicrotask(root, now());
}
}

// TODO: Rename to something else. This isn't a generic callback queue anymore.
Expand Down Expand Up @@ -275,9 +266,6 @@ function scheduleTaskForRootDuringMicrotask(
// rendering task right before we yield to the main thread. It should never be
// called synchronously.
//
// TODO: Unless enableDeferRootSchedulingToMicrotask is off. We need to land
// that ASAP to unblock additional features we have planned.
//
// This function also never performs React work synchronously; it should
// only schedule work to be performed later, in a separate task or microtask.

Expand Down
118 changes: 0 additions & 118 deletions packages/react-reconciler/src/ReactFiberSyncTaskQueue.js

This file was deleted.

Loading

0 comments on commit 9faeced

Please sign in to comment.