[Flight][Float] Preinitialize module imports during SSR #27314
Conversation
facebook-github-bot
added
the
React Core Team
gnoff
force-pushed
the
preinit-imports-webpack-esm
branch
from
a6011c2
to
ebd48ac
Compare
| const webpackGetChunkFilename = __webpack_require__.u; | ||
| __webpack_require__.u = function (chunkId: string) { | ||
| const flightChunk = chunkMap.get(chunkId); | ||
| if (flightChunk !== undefined) { | ||
| return flightChunk; | ||
| } | ||
| return webpackGetChunkFilename(chunkId); | ||
| }; | ||
|
|
||
| export function loadChunk(chunkId: string, filename: string): Promise<mixed> { | ||
| chunkMap.set(chunkId, filename); | ||
| return __webpack_chunk_load__(chunkId); | ||
| } |
There was a problem hiding this comment.
@sebmarkbage why did we think we needed chunkId still on the client? With this implementation I can just send the filenames. I believe if there is a dynamic import for module Foo and module Foo is also in a client component chunk the dynamic import gets it's own chunk I think. webpack tries to load the dynamic chunk it'll have a different chunkId than the chunk used for client components. This is unfortunate b/c the code might already be on the client and if there were module state it wouldn't be shared across both instances. However if this is just how the bundling has to be there is no sense in serializing the chunkIds for client component imports because we can just look up the chunk in chunkMap using the filename rather than the chunkId (by passing the filename into __webpack_chunk_load__
There was a problem hiding this comment.
I think the answer I don't think Webpack does duplicate the modules. If it does, then we need to fix that. Maybe there's a bug in Next.js but I wouldn't expect the prototype implementation do that. Can you confirm?
There was a problem hiding this comment.
I confirmed you are correct
gnoff
force-pushed
the
preinit-imports-webpack-esm
branch
2 times, most recently
from
bf0c510
to
7f8d6d1
Compare
fixtures/flight/server/global.js
Outdated
| await virtualFs.readFile( | ||
| path.join(buildPath, 'react-ssr-manifest.json'), | ||
| path.join(buildPath, 'react-ssr-bundle-config.json'), |
There was a problem hiding this comment.
The name manifest comes from the Webpack usage. That just leaked into our internal SSRManifest name. Renaming it is a bit the tail wagging the dog. We shouldn't try to change the naming convention for these files which are still just manifests.
There was a problem hiding this comment.
In other words, our SSRManifest was a Module Map living in a Manifest file. Now it's a Bundler Config living in a Manifest file. It doesn't change that the outer thing is still a manifest.
There was a problem hiding this comment.
I updated the filename but also renamed our type SSRManifest to SSRModuleMap since that is what it is. We can go back to the manifest type if we consolidate ModuleLoading with the ModuleMap as you questioned below however since you previously suggested doing the opposite I'll await further consideration there.
fixtures/flight/server/global.js
Outdated
| } | ||
| root = createFromNodeStream(rscResponse, ssrBundleConfig.ssrManifest, { | ||
| moduleLoading: ssrBundleConfig.moduleLoading, | ||
| }); |
There was a problem hiding this comment.
Does this suggest that this should just be a single argument option that has both in it?
There was a problem hiding this comment.
That's what my original implementation had and your feedback at that time was preinitialization and ssr module mapping were unrelated concepts and I should separate them :)
The moduleLoading is technically optional whereas the ssrManifest is a must so I can see the argument they are separate.
There was a problem hiding this comment.
There's an internal concept and an external concept. This is also related to the name of the type. Almost always we refer to the internal type as one thing but the exported type (which is the name we recommend DefinitelyTyped to use) could be aliased to another name.
I wonder if that's also the case here. Internally they can be two separate concepts and such the configs have to deal with them separately but we can join them here as a single input. Especially if we expect the Webpack plugin to generate it. I don't think I expected that we'd generate it originally.
| @@ -0,0 +1,45 @@ | |||
| /** | |||
There was a problem hiding this comment.
This file should maybe be called ClientReferenceMetaData or maybe we go away from the ClientReference naming all together and call it ImportMetaData or something.
Because there is a notion of a ClientReference and ServerReference - the reified thing (e.g. proxy or real thing) - which is not this. And there's no code in this file related to it, and there shouldn't be because that code can't be shared before server/client.
So this file shouldn't be called ReactFlightClientReference.js.
There was a problem hiding this comment.
yeah this makes sense to me
gnoff
force-pushed
the
preinit-imports-webpack-esm
branch
4 times, most recently
from
f34a29c
to
4cc6990
Compare
gnoff
force-pushed
the
preinit-imports-webpack-esm
branch
9 times, most recently
from
1895b38
to
19237fb
Compare
gnoff
force-pushed
the
preinit-imports-webpack-esm
branch
3 times, most recently
from
0df428a
to
c16a3b1
Compare
…tched before bootstrap in the browser
gnoff
force-pushed
the
preinit-imports-webpack-esm
branch
from
c16a3b1
to
77fed1f
Compare
Currently when we SSR a Flight response we do not emit any resources for module imports. This means that when the client hydrates it won't have already loaded the necessary scripts to satisfy the Imports defined in the Flight payload which will lead to a delay in hydration completing. This change updates `react-server-dom-webpack` and `react-server-dom-esm` to emit async script tags in the head when we encounter a modules in the flight response. To support this we need some additional server configuration. We need to know the path prefix for chunk loading and whether the chunks will load with CORS or not (and if so with what configuration). DiffTrain build for [701ac2e](701ac2e)
stacked on #27314 Turbopack requires a different module loading strategy than Webpack and as such this PR implements a new package `react-server-dom-turbopack` which largely follows the `react-server-dom-webpack` but is implemented for this new bundler
React upstream changes: - facebook/react#27439 - facebook/react#26763 - facebook/react#27434 - facebook/react#27433 - facebook/react#27424 - facebook/react#27428 - facebook/react#27427 - facebook/react#27315 - facebook/react#27314 - facebook/react#27400 - facebook/react#27421 - facebook/react#27419 - facebook/react#27418
Today when we hydrate an SSR'd RSC response on the client we encounter import chunks which initiate code loading for client components. However we only start fetching these chunks after hydration has begun which is necessarily after the initial chunks for the entrypoint have loaded. React has upstream changes that need to land which will preinitialize the rendered chunks for all client components used during the SSR pass. This will cause a `<script async="" src... />` tag to be emitted in the head for each chunk we need to load during hydration which allows the browser to start fetching these resources even before the entrypoint has started to execute. Additionally the implementation for webpack and turbopack is different enough that there will be a new `react-server-dom-turbopack` package in the React repo which should be used when using Turbopack with Next. This PR also removes a number of patches to React src that proxy loading (`__next_chunk_load__`) and bundler requires (`__next_require__`) through the `globalThis` object. Now the react packages can be fully responsible for implementing chunk loading and all Next needs to do is supply the necessary information such as chunk prefix and crossOrigin attributes necessary for this loading. This information is produced as part of the client-manifest by either a Webpack plugin or Turbopack. Additionally any modifications to the chunk filename that were previously done at runtime need to be made in the manifest itself now. This means we need to encode the deployment id for skew protection and encode the filename to make it match our static path matching (and resolutions on s3) when using `[` and `]` segment characters. There are a few followup items to consider in later PRs 1. we currently bundle a node and edge version of react-server-dom-webpack/client. The node version has an implementation for busboy whereas the edge version does not. Next is currently configured to use busboy when handling a fetch action sent as multipart with a node runtime. Ideally we'd only bundle the one platform we are buliding for but some additional refactoring to support better forking is possibly required here This PR also updates react from 09285d5a7 to d900fadbf. ### React upstream changes - facebook/react#27439 - facebook/react#26763 - facebook/react#27434 - facebook/react#27433 - facebook/react#27424 - facebook/react#27428 - facebook/react#27427 - facebook/react#27315 - facebook/react#27314 - facebook/react#27400 - facebook/react#27421 - facebook/react#27419 - facebook/react#27418
Based on facebook/react#27314 This feature requires `AsyncLocalStorage` to be available in the edge environment via the `node:async_hooks` module. This is the case for [Vercel](https://vercel.com/docs/functions/edge-functions/edge-runtime#compatible-node.js-modules) as well as [Cloudflare](https://developers.cloudflare.com/workers/runtime-apis/nodejs/asynclocalstorage/). fixes #17
Based on facebook/react#27314 This feature requires `AsyncLocalStorage` to be available in the edge environment via the `node:async_hooks` module. This is the case for [Vercel](https://vercel.com/docs/functions/edge-functions/edge-runtime#compatible-node.js-modules) as well as [Cloudflare](https://developers.cloudflare.com/workers/runtime-apis/nodejs/asynclocalstorage/). fixes #17
We have an unresolved conflict where the Flight client wants to execute inside Fizz to emit side-effects like preloads (which can be early) into that stream. However, the FormState API requires the state to be passed at the root, so if you're getting that through the RSC payload it's a Catch 22. #27314 used a hack to mutate the form state array to fill it in later, but that doesn't actually work because it's not always an array. It's sometimes null like if there wasn't a POST. This lead to a bunch of hydration errors - which doesn't have the best error message for this case neither. It probably should error with something that specifies that it's form state. This fixes it by teeing the stream into two streams and consuming it with two Flight clients. One to read the form state and one to emit side-effects and read the root.
Currently when we SSR a Flight response we do not emit any resources for module imports. This means that when the client hydrates it won't have already loaded the necessary scripts to satisfy the Imports defined in the Flight payload which will lead to a delay in hydration completing. This change updates `react-server-dom-webpack` and `react-server-dom-esm` to emit async script tags in the head when we encounter a modules in the flight response. To support this we need some additional server configuration. We need to know the path prefix for chunk loading and whether the chunks will load with CORS or not (and if so with what configuration).
stacked on facebook#27314 Turbopack requires a different module loading strategy than Webpack and as such this PR implements a new package `react-server-dom-turbopack` which largely follows the `react-server-dom-webpack` but is implemented for this new bundler
We have an unresolved conflict where the Flight client wants to execute inside Fizz to emit side-effects like preloads (which can be early) into that stream. However, the FormState API requires the state to be passed at the root, so if you're getting that through the RSC payload it's a Catch 22. facebook#27314 used a hack to mutate the form state array to fill it in later, but that doesn't actually work because it's not always an array. It's sometimes null like if there wasn't a POST. This lead to a bunch of hydration errors - which doesn't have the best error message for this case neither. It probably should error with something that specifies that it's form state. This fixes it by teeing the stream into two streams and consuming it with two Flight clients. One to read the form state and one to emit side-effects and read the root.
Currently when we SSR a Flight response we do not emit any resources for module imports. This means that when the client hydrates it won't have already loaded the necessary scripts to satisfy the Imports defined in the Flight payload which will lead to a delay in hydration completing. This change updates `react-server-dom-webpack` and `react-server-dom-esm` to emit async script tags in the head when we encounter a modules in the flight response. To support this we need some additional server configuration. We need to know the path prefix for chunk loading and whether the chunks will load with CORS or not (and if so with what configuration). DiffTrain build for commit 701ac2e.
We have an unresolved conflict where the Flight client wants to execute inside Fizz to emit side-effects like preloads (which can be early) into that stream. However, the FormState API requires the state to be passed at the root, so if you're getting that through the RSC payload it's a Catch 22. #27314 used a hack to mutate the form state array to fill it in later, but that doesn't actually work because it's not always an array. It's sometimes null like if there wasn't a POST. This lead to a bunch of hydration errors - which doesn't have the best error message for this case neither. It probably should error with something that specifies that it's form state. This fixes it by teeing the stream into two streams and consuming it with two Flight clients. One to read the form state and one to emit side-effects and read the root. DiffTrain build for commit 6a44f35.
Currently when we SSR a Flight response we do not emit any resources for module imports. This means that when the client hydrates it won't have already loaded the necessary scripts to satisfy the Imports defined in the Flight payload which will lead to a delay in hydration completing.
This change updates
react-server-dom-webpackandreact-server-dom-esmto emit async script tags in the head when we encounter a modules in the flight response.To support this we need some additional server configuration. We need to know the path prefix for chunk loading and whether the chunks will load with CORS or not (and if so with what configuration).