[Flight] Schedule work in a microtask #29491
Merged
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
gnoff
commented
May 23, 2024
Comment on lines
20
to
22
| throw new Error( | ||
| 'Legacy React Server builds should not be using scheduleMicrotask. This is a bug in React.', | ||
| ); |
There was a problem hiding this comment.
Should environments that previously scheduled all work sync just continue to do sync work? I Should probably rename to scheduleWorkInCurrentTask since it otherwise lies about what it does.
I made this error because we have no legacy builds of flight which is currently the only place this new API is used but this feels pretty inconsistent to me.
Comment on lines
25
to
27
| : callback => { | ||
| Promise.resolve(null).then(callback).catch(handleErrorInNextTick); | ||
| }; |
There was a problem hiding this comment.
unlike in Fiber where there is a feature test for Promise I assume here that you must be using an environment with Promise globals. At this point I think we can safely just say React requires promises and since 19 is about to ship maybe we make this breaking change in Fiber too
Comment on lines
25
to
26
| export const scheduleMicrotask = queueMicrotask; | ||
|
|
There was a problem hiding this comment.
supported since Node 11 so just not bothering to fall back to Promise
packages/react-server/src/forks/ReactServerStreamConfig.dom-fb-experimental.js
Outdated
Show resolved
Hide resolved
gnoff
force-pushed
the
microtask-work-in-flight
branch
2 times, most recently
from
4ecb5bc
to
87178ee
Compare
gnoff
force-pushed
the
microtask-work-in-flight
branch
from
87178ee
to
6947934
Compare
gnoff
force-pushed
the
microtask-work-in-flight
branch
from
6947934
to
9b366ff
Compare
This was referenced Jun 1, 2024
gnoff
force-pushed
the
microtask-work-in-flight
branch
from
9b366ff
to
b9a8668
Compare
sebmarkbage
approved these changes
Jun 2, 2024
gnoff
force-pushed
the
microtask-work-in-flight
branch
from
b9a8668
to
ab5cea4
Compare
gnoff
force-pushed
the
microtask-work-in-flight
branch
from
ab5cea4
to
c865358
Compare
Flight pings much more often than Fizz because async function components will always take at least a microtask to resolve . Rather than scheduling this work as a new macrotask Flight now schedules pings in a microtask. This allows more microta sks to ping before actually doing a work flush but doesn't force the vm to sping up a new task which is quite common give n the nature of Server Components
gnoff
force-pushed
the
microtask-work-in-flight
branch
from
c865358
to
c2bbe63
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Jun 6, 2024
Stacked on #29551 Flight pings much more often than Fizz because async function components will always take at least a microtask to resolve . Rather than scheduling this work as a new macrotask Flight now schedules pings in a microtask. This allows more microtasks to ping before actually doing a work flush but doesn't force the vm to spin up a new task which is quite common give n the nature of Server Components DiffTrain build for commit 1e1e5cd.
gnoff
added a commit
that referenced
this pull request
Jun 6, 2024
Stacked on #29491 Previously if you aborted during a render the currently rendering task would itself be aborted which will cause the entire model to be replaced by the aborted error rather than just the slot currently being rendered. This change updates the abort logic to mark currently rendering tasks as aborted but allowing the current render to emit a partially serialized model with an error reference in place of the current model. The intent is to support aborting from rendering synchronously, in microtasks (after an await or in a .then) and in lazy initializers. We don't specifically support aborting from things like proxies that might be triggered during serialization of props
github-actions bot
pushed a commit
that referenced
this pull request
Jun 6, 2024
Stacked on #29491 Previously if you aborted during a render the currently rendering task would itself be aborted which will cause the entire model to be replaced by the aborted error rather than just the slot currently being rendered. This change updates the abort logic to mark currently rendering tasks as aborted but allowing the current render to emit a partially serialized model with an error reference in place of the current model. The intent is to support aborting from rendering synchronously, in microtasks (after an await or in a .then) and in lazy initializers. We don't specifically support aborting from things like proxies that might be triggered during serialization of props DiffTrain build for commit c4b433f.
gnoff
added a commit
to gnoff/react
that referenced
this pull request
Aug 21, 2024
In facebook#29491 I updated the work scheduler for Flight to use microtasks to perform work when something pings. This is useful but it does have some downsides in terms of our ability to do task prioritization. Additionally the initial work is not instantiated using a microtask which is inconsistent with how pings work. In this change I update the scheduling logic to use microtasks consistently for prerenders and use regular tasks for renders both for the initial work and pings.
gnoff
added a commit
to gnoff/react
that referenced
this pull request
Aug 21, 2024
In facebook#29491 I updated the work scheduler for Flight to use microtasks to perform work when something pings. This is useful but it does have some downsides in terms of our ability to do task prioritization. Additionally the initial work is not instantiated using a microtask which is inconsistent with how pings work. In this change I update the scheduling logic to use microtasks consistently for prerenders and use regular tasks for renders both for the initial work and pings.
gnoff
added a commit
that referenced
this pull request
Aug 21, 2024
In #29491 I updated the work scheduler for Flight to use microtasks to perform work when something pings. This is useful but it does have some downsides in terms of our ability to do task prioritization. Additionally the initial work is not instantiated using a microtask which is inconsistent with how pings work. In this change I update the scheduling logic to use microtasks consistently for prerenders and use regular tasks for renders both for the initial work and pings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stacked on #29551
Flight pings much more often than Fizz because async function components will always take at least a microtask to resolve . Rather than scheduling this work as a new macrotask Flight now schedules pings in a microtask. This allows more microtasks to ping before actually doing a work flush but doesn't force the vm to spin up a new task which is quite common give n the nature of Server Components