fix(modernjs): quiet valid stream SSR splitChunks config#4851
fix(modernjs): quiet valid stream SSR splitChunks config#4851BleedingDev wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 0b08935 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".
BleedingDev
force-pushed
the
codex/fix-modernjs-splitchunks-warning
branch
from
cfbb0f5 to
d7001da
Compare
BleedingDev
force-pushed
the
codex/fix-modernjs-splitchunks-warning
branch
from
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 👍 / 👎.
1 participant
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