-
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
[Flight] Preinitialize Imports #26795
Conversation
082f23c
to
f9d21cf
Compare
a0dab99
to
04f0e2b
Compare
@@ -37,12 +37,12 @@ export function createServerReference<A: Iterable<any>, T>( | |||
} | |||
|
|||
export type Options = { | |||
moduleMap?: $NonMaybeType<SSRManifest>, | |||
bundleConfig?: $NonMaybeType<BundleConfig>, |
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.
"bundleConfig" inside an "options" is an unnecessary abstraction. The notion of a bundle config is an internal concept. The public API is just options. In other words, the Options object should just get another option added to it.
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 currently have chunkLoading as a required argument to the various createFrom...
methods.
My thinking is that properly handling chunkLoading is so important it shouldn't be optional in the environments where it makes sense (Edge, Node). I considered defaulting it but there is no actually good default since a simple webpack config change would easily break it.
I could also infer it from webpack itself but that only works if you bundle react-server-dom-webpack which isn't necessarily what you want and it wouldn't help in Node's case
const crossOrigin = bundlerConfig.chunkLoading.crossOrigin; | ||
const chunks = metadata[CHUNKS]; | ||
for (let i = 1; i < chunks.length; i += 2) { | ||
preinitModulesForSSR(prefix + chunks[i], crossOrigin); |
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.
Like we discussed, this should probably move out to an explicit call from FlightClient that's not part of "resolving". Maybe it's not part of bundler config at all but maybe more tied to the DOM specific config?
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.
Addressed this in the latest push. The layering is a little weird because we have an opaque type for ClientReferenceMetadata in the bundler config so when FlightClient calls into some host specific function with the metadata the only file that can unwrap that is the bundler config
The layering is now
bundler config exposes a proxy method that unwraps the metadata type
destination config exposes a method that implements what we do with webpack chunks when preparing the destination (in this case preiniting)
host config exposes the host specific implementation of preinitialization
The bundler config layer is not necessary but for the opaque type and we could do something else like make a de-opaquing identity function or just type casting but this all gets flattened into a single inline call so I think it is reasonable this way
d53877d
to
8d3458d
Compare
856057e
to
e375e2b
Compare
3aaa02b
to
dfd32bc
Compare
d6db457
to
b3545a2
Compare
…the files but it adds maintenance burden in the inlinedHostConfigs whenever things change there. Going to make these configs opaque mixed types to quiet flow since no entrypoints use the flight code
…tched before bootstrap in the browser
Implements
prepareDestinationForModule
forreact-server-dom-webpack
andreact-server-dom-esm
.Implements a new package
react-server-dom-turbopack
which largely mirrors webpack with some custom chunk-loading implemention.There are couple utility PRs that I'll probably land separately. It'll be easiest to follow commit by commit