Skip to content

Commit

Permalink
Move update scheduling to microtask (#26512)
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 31, 2023
1 parent 8310854 commit 09c8d25
Show file tree
Hide file tree
Showing 19 changed files with 557 additions and 376 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ describe('ProfilingCache', () => {
2 => 0,
},
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 0,
"updaters": [
{
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

0 comments on commit 09c8d25

Please sign in to comment.