-
Notifications
You must be signed in to change notification settings - Fork 47k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preload imports #26664
Preload imports #26664
Conversation
} | ||
root = createFromNodeStream(rscResponse, moduleMap); | ||
return root; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like the implementation of what @sebmarkbage mentioned in this tweet. Can you explain why this change is needed for preloading the client chunks? Does it lead to HTML chunks being emitted earlier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flight rows are processed outside of a render. For directives coming from Flight like instructions to preload something we still need to be able to associate these outside of render things to a specific request in SSR. By creating the Flight client during a Fizz render we can associate the flight client rows to a Request. If the environment supports AsyncLocalStorage then even async code (really anything outside of a render) can still see this Request. In environments that don’t support asynclocalstorage flight directives will get dropped because there is no associated Fizz request to refer to when not rendering. Currently all directives are optimistic so if we lose some during SSR it’s not going to break anything just be slower. The hope is that we get an AsyncContext or AsyncLocalStorage in every runtime that can do SSR and this will just be the norm. However none of this would work if you just created a random Flight client out of band because the client wouldn’t know where to route anything. We could take other approaches like encoding the directives into the value that will be rendered but that adds a lot of complexity and we think this pattern is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for taking the time to write this thorough explanation!
So, we basically need to create the following sequence for preload resources to be associated with the request and thus rendered (I'm just sharing my debugging notes here, in case anyone else wants to follow along):
renderToPipeableStream
createRequest
(with resources)- set current dispatcher to
ReactDOMServerDispatcher
setCurrentResources
(with resources from request)- ... *hand waiving* render
Root
createFromNodeStream
processFullRow
(caseI
)resolveModule
preloadModule
preloadModuleForSSR
ReactDOMCurrentDispatcher.current.preload
(which is nowReactDOMServerDispatcher
)- create preload resource with link chunks and add to
resources.explicitScriptPreloads
- flush link chunks in
writePreamble
If createFromNodeStream
were to be called outside of Root
, as was the case before in the Flight fixture, the current dispatcher would be (the default?) ReactDOMClientDispatcher
, and the preload
call would subsequently fail with ReferenceError: document is not defined
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some nuance here maybe not captured in this linear list but in a handwavy way yeah this is what is happening.
What is important to happen during render is the flight client construction, the actual flight render can be initiated outside of the Fizz render.
Another note is that processFullRow doesn't actually run during the Fizz render even when the flight client is constructed during a Fizz render
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean to change the part where teh ReactDOMClientDispatcher
is set during SSR. This should only be set when running in a browser so if you fail to create the flight client during a Fizz render it just noops instead of failing on a missing document
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. 👍
if (ssrChunks) { | ||
for (let i = 0; i < ssrChunks.length; i++) { | ||
const href = ssrChunks[i]; | ||
preloadModuleForSSR(href); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I'm reading through this pull request, I can see that this is intended for Float to render <link rel="preload" as="script" href="/some-client-chunk.js" />
. As someone who is not as deeply involved in the project, I thought it might be helpful to share an outsider's perspective. From my point of view, renaming ssrChunks
to clientChunks
and preloadModuleForSSR
to preloadModuleForHydration
could potentially make the code more understandable without context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the general intent of the code. I’m currently reworking the implementation significantly so none of these names are likely to stick around
This is in preparation for the upcoming React feature of adding preload link elements for client chunks that are needed for hydration. See the following discussion for more details: facebook/react#26664 (comment)
Supporting Float methods such as ReactDOM.preload() are challenging for flight because it does not have an easy means to convey direct executions in other environments. Because the flight wire format is a JSON-like serialization that is expected to be rendered it currently only describes renderable elements. We need a way to convey a function invocation that gets run in the context of the client environment whether that is Fizz or Fiber. Fiber is somewhat straightforward because the HostDispatcher is always active and we can just have the FlightClient dispatch the serialized directive. Fizz is much more challenging becaue the dispatcher is always scoped but the specific request the dispatch belongs to is not readily available. Environments that support AsyncLocalStorage (or in the future AsyncContext) we will use this to be able to resolve directives in Fizz to the appropriate Request. For other environments directives will be elided. Right now this is pragmatic and non-breaking because all directives are opportunistic and non-critical. If this changes in the future we will need to reconsider how widespread support for async context tracking is. For Flight, if AsyncLocalStorage is available Float methods can be called before and after await points and be expected to work. If AsyncLocalStorage is not available float methods called in the sync phase of a component render will be captured but anything after an await point will be a noop. If a float call is dropped in this manner a DEV warning should help you realize your code may need to be modified.
superceded by #26795 |
Preloads Import rows during SSR so browser can start to download before JS has loaded