fix(modernjs): quiet valid stream SSR splitChunks config#4851
fix(modernjs): quiet valid stream SSR splitChunks config#4851BleedingDev wants to merge 7 commits into
Conversation
🦋 Changeset detectedLatest commit: 7d46c22 The changes in this PR will be included in the next version bump. This PR includes changesets to release 47 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfbb0f58ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
cfbb0f5 to
d7001da
Compare
d7001da to
4ae9519
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ae9519251
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const currentChunks = splitChunkConfig.chunks; | ||
| splitChunkConfig.chunks = 'async'; | ||
| logger.warn( | ||
| `splitChunks.chunks = async is not allowed with stream SSR mode, it will auto changed to "async"`, | ||
| ); | ||
| if (currentChunks !== 'async') { |
There was a problem hiding this comment.
When a user's config has top-level splitChunks.chunks: 'async' but any cache group sets its own chunks: 'all'/'initial', this guard suppresses the stream-SSR warning even though cache-group chunks overrides the top-level setting. This code only rewrites splitChunkConfig.chunks and leaves those cache-group overrides untouched, so the incompatible split remains silently (same mirror in modernjs-v3). Please include cache-group chunk overrides in the check or normalize them before skipping the warning.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 948dc693d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `splitChunks.chunks = async is not allowed with stream SSR mode, it will auto changed to "async"`, | ||
| ); | ||
|
|
||
| for (const cacheGroup of Object.values(splitChunkConfig.cacheGroups)) { |
There was a problem hiding this comment.
When stream SSR uses top-level splitChunks.chunks: 'async' but splitChunks.fallbackCacheGroup.chunks is set to 'all' or 'initial', this new pass only scans cacheGroups, so shouldWarn can stay false while the fallback cache group still overrides chunk selection. Webpack documents fallbackCacheGroup.chunks as part of split-chunk chunk selection (https://webpack.js.org/plugins/split-chunks-plugin/#splitchunkschunks), so this can silently leave the same incompatible initial/all split in place; the mirrored loop in packages/modernjs-v3/src/cli/configPlugin.ts needs the same handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d9e7b963c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| splitChunkConfig.cacheGroups | ||
| typeof splitChunkConfig === 'object' | ||
| ) { | ||
| let shouldWarn = splitChunkConfig.chunks !== 'async'; |
There was a problem hiding this comment.
When a stream-SSR client build leaves optimization.splitChunks.chunks unset, the bundler default is already async, but this initializer treats the resulting undefined as a change-worthy value and emits the warning anyway. That makes otherwise valid default splitChunks configs noisy again; the mirrored logic in packages/modernjs-v3/src/cli/configPlugin.ts needs the same default-as-async handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 299cb93fef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'chunks' in cacheGroup && | ||
| cacheGroup.chunks !== 'async' | ||
| ) { | ||
| cacheGroup.chunks = 'async'; |
There was a problem hiding this comment.
When an SSR client config uses a function-valued cacheGroups.foo.chunks that already limits the group to async chunks, this condition treats the function as incompatible and replaces it with 'async'. That broadens the cache group to every async chunk and drops the user's chunk-splitting logic; elsewhere in the repo (ContainerPlugin.patchChunkSplit) function filters are explicitly supported. Please wrap or preserve function filters instead of overwriting them, and make the same adjustment in the modernjs-v3 mirror.
Useful? React with 👍 / 👎.
@module-federation/devtools
@module-federation/cli
create-module-federation
@module-federation/dts-plugin
@module-federation/enhanced
@module-federation/error-codes
@module-federation/esbuild
@module-federation/managers
@module-federation/manifest
@module-federation/metro
@module-federation/metro-plugin-rnc-cli
@module-federation/metro-plugin-rnef
@module-federation/metro-plugin-rock
@module-federation/modern-js
@module-federation/modern-js-v3
@module-federation/native-federation-tests
@module-federation/native-federation-typescript
@module-federation/nextjs-mf
@module-federation/node
@module-federation/observability-plugin
@module-federation/playground
@module-federation/retry-plugin
@module-federation/rsbuild-plugin
@module-federation/rspack
@module-federation/rspress-plugin
@module-federation/runtime
@module-federation/runtime-core
@module-federation/runtime-tools
@module-federation/sdk
@module-federation/storybook-addon
@module-federation/third-party-dts-extractor
@module-federation/treeshake-frontend
@module-federation/treeshake-server
@module-federation/typescript
@module-federation/utilities
@module-federation/webpack-bundler-runtime
@module-federation/bridge-react
@module-federation/bridge-react-webpack-plugin
@module-federation/bridge-shared
@module-federation/bridge-vue3
@module-federation/inject-external-runtime-core-plugin
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c454fc3670
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let shouldWarn = | ||
| typeof splitChunksValue === 'string' && splitChunksValue !== 'async'; |
There was a problem hiding this comment.
When stream SSR is enabled and a user supplies a top-level function filter such as splitChunks.chunks = () => true, this new string-only guard neither rewrites nor warns, so initial chunks can still be split even though this plugin is trying to force async-only splitting. Fresh evidence is the new typeof ... === 'string' check, which skips top-level predicates that the previous implementation normalized when cache groups existed; the repo also treats a predicate returning true as equivalent to all in ContainerPlugin.patchChunkSplit. Please wrap function filters with the async-only constraint, or reject/warn for functions that cannot be proven async, and mirror the fix in the Modern.js v3 copy.
Useful? React with 👍 / 👎.
Bundle Size Report12 package(s) changed, 30 unchanged. Package dist + ESM entry
Bundle targets
Consumer scenarios
Total dist (raw): 35.70 MB (+7.0 kB (+0.0%)) Bundle sizes are generated with rslib (Rspack). Package-root metrics preserve the historical report. Tracked subpath exports such as |
|
I found one issue that should be fixed before merging. The stream SSR splitChunks normalization now only handles string values. If a user provides a function filter, such as This affects both Modern.js adapters:
The new tests also lock in this behavior by expecting function filters to be preserved. Please wrap function filters with an async-only guard, or warn/reject them if they cannot be safely constrained, and mirror the change in both adapters. |
2 participants
Summary
asyncsplitChunks.chunksis alreadyasyncValidation
pnpm --filter @module-federation/modern-js-v3 run testpnpm --filter @module-federation/modern-js run testpnpm exec prettier --check packages/modernjs-v3/src/cli/configPlugin.ts packages/modernjs-v3/src/cli/configPlugin.spec.ts packages/modernjs/src/cli/configPlugin.ts packages/modernjs/src/cli/configPlugin.spec.ts .changeset/quiet-stream-ssr-splitchunks.mdpnpm exec turbo run build --filter=@module-federation/modern-js... --filter=@module-federation/modern-js-v3... --concurrency=4