-
Notifications
You must be signed in to change notification settings - Fork 46.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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.
- Loading branch information
Showing
10 changed files
with
59 additions
and
386 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 0 additions & 118 deletions
118
packages/react-reconciler/src/ReactFiberSyncTaskQueue.js
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.