fix(esm-library): handle CSS modules in preserveModules#13670
fix(esm-library): handle CSS modules in preserveModules#13670
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73feb3d522
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Fixes output.library.preserveModules behavior when CSS-only modules/chunks are present, preventing ESM library rendering from emitting JS imports/requires for chunks that have no JavaScript asset and enabling per-chunk CSS filename overrides to take effect (including with CssExtractRspackPlugin).
Changes:
- Classify
preserveModulessplitting by modulesource_types, preserving.cssextension and applyingchunk.css_filename_templatefor CSS output. - Skip CSS-only modules/chunks during ESM render so JS output no longer references CSS-only chunks as JS imports/requires.
- Make
CssExtractRspackPluginrespectchunk.css_filename_templateand add new ESM output test fixtures for native CSS + extract-css.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rspack_plugin_extract_css/src/plugin.rs | Honors per-chunk CSS filename overrides (chunk.css_filename_template) when emitting extracted CSS assets. |
| crates/rspack_plugin_esm_library/src/render.rs | Skips emitting JS requires/imports for CSS-only modules and chunks in ESM rendering. |
| crates/rspack_plugin_esm_library/src/preserve_modules.rs | Splits modules by kind (JS vs CSS), preserves .css names, and sets css_filename_template for CSS chunks. |
| crates/rspack_core/src/chunk.rs | Adds set_css_filename_template setter to support per-chunk CSS naming updates. |
| tests/rspack-test/esmOutputCases/preserve-modules/css/test.config.js | New esm-output case config for native CSS + preserveModules. |
| tests/rspack-test/esmOutputCases/preserve-modules/css/rspack.config.js | Enables experiments.css and preserveModules for native CSS fixture. |
| tests/rspack-test/esmOutputCases/preserve-modules/css/src/index.js | Imports CSS and a nested JS module for the native CSS fixture. |
| tests/rspack-test/esmOutputCases/preserve-modules/css/src/style.css | Native CSS fixture stylesheet. |
| tests/rspack-test/esmOutputCases/preserve-modules/css/src/components/button/index.js | Nested JS module used by the native CSS fixture. |
| tests/rspack-test/esmOutputCases/preserve-modules/css/src/components/button/style.css | Nested native CSS fixture stylesheet. |
| tests/rspack-test/esmOutputCases/preserve-modules/css/snapshots/esm.snap.txt | Snapshot for native CSS preserveModules fixture. |
| tests/rspack-test/esmOutputCases/preserve-modules/css-extract/test.config.js | New esm-output case config for extract-css + preserveModules. |
| tests/rspack-test/esmOutputCases/preserve-modules/css-extract/rspack.config.js | Configures CssExtractRspackPlugin and preserveModules for extract-css fixture. |
| tests/rspack-test/esmOutputCases/preserve-modules/css-extract/src/index.js | Imports CSS and a nested JS module for the extract-css fixture. |
| tests/rspack-test/esmOutputCases/preserve-modules/css-extract/src/style.css | Extract-css fixture stylesheet. |
| tests/rspack-test/esmOutputCases/preserve-modules/css-extract/src/components/button/index.js | Nested JS module used by the extract-css fixture. |
| tests/rspack-test/esmOutputCases/preserve-modules/css-extract/src/components/button/style.css | Nested extract-css fixture stylesheet. |
| tests/rspack-test/esmOutputCases/preserve-modules/css-extract/snapshots/esm.snap.txt | Snapshot for extract-css preserveModules fixture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/rspack-test/esmOutputCases/preserve-modules/css-extract/test.config.js
Show resolved
Hide resolved
Merging this PR will degrade performance by 1.17%
Performance Changes
Comparing |
Rsdoctor Bundle Diff AnalysisFound 6 projects in monorepo, 0 projects with changes. 📊 Quick Summary
Generated by Rsdoctor GitHub Action |
📦 Binary Size-limit
🎉 Size decreased by 4.97KB from 49.39MB to 49.38MB (⬇️0.01%) |
a2e272a to
c085726
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
49640dc to
5f69f8b
Compare
Deploying rspack with
|
| Latest commit: |
2df2b07
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://47724d76.rspack-v2.pages.dev |
| Branch Preview URL: | https://fy-friendly-volhard.rspack-v2.pages.dev |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2df2b07c2e
ℹ️ 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 base_name: String = if let Some(extension) = file_path.extension() { | ||
| let suffix = format!(".{}", extension.to_string_lossy()); | ||
| file_path_str | ||
| .strip_suffix(&suffix) | ||
| .unwrap_or(&file_path_str) |
There was a problem hiding this comment.
Preserve unique chunk names across file extensions
Stripping the source extension unconditionally makes foo.js and foo.css both resolve to the same base_name (foo) under preserveModules. That introduces duplicate chunk names/ids for different chunks, and downstream code assumes ids are unique (NamedChunkIdsPlugin uses chunk.name() directly and after_code_generation stores id -> ukey with overwrite). In projects that colocate files like index.js + index.css, JS placeholders can resolve to the CSS-only chunk and later fail in process_assets with "chunk <id> should have at least one file".
Useful? React with 👍 / 👎.
| let file_path_str = file_path | ||
| .to_slash() | ||
| .expect("relative path should be valid UTF-8") | ||
| .into_owned(); |
There was a problem hiding this comment.
Avoid panicking on non-UTF8 module paths
to_slash().expect("relative path should be valid UTF-8") introduces a hard panic when a preserved module path contains non-UTF8 bytes (valid on Unix filesystems). The previous implementation used lossy conversion and continued; with this change, compilation can abort unexpectedly for such repositories instead of returning a normal diagnostic.
Useful? React with 👍 / 👎.
2df2b07 to
35fbb90
Compare
35fbb90 to
885ce36
Compare
CSS modules (native `experiments.css` or mini-css-extract) under `output.library.preserveModules` caused either "chunk xxx should have at least one file" or "Multiple assets emit different content to the same filename". preserveModules set the JS filename_template on CSS chunks, left chunks nameless so multiple CSS files collapsed onto `.css`, and the ESM render phase emitted `import "__RSPACK_ESM_CHUNK_<id>"` placeholders pointing at CSS-only chunks with no JS file. Classify modules in preserve_modules by source_types and set css_filename_template for CSS modules (preserving the `.css` extension and source path), skip CSS-only modules/chunks in the ESM render paths that emit JS-side requires / cross-chunk imports, and make CssExtractRspackPlugin honor chunk.css_filename_template so preserveModules can override its per-chunk filename.
885ce36 to
b3d5c82
Compare
Summary
output.library.preserveModulespanic when CSS modules are involved — both native CSS (experiments.css: true) andCssExtractRspackPlugin. The build previously died with "chunk xxx should have at least one file" or "Multiple assets emit different content to the same filename".preserve_modulesbysource_types, preserve the.cssextension and source path for CSS modules, and setcss_filename_templateinstead of the JSfilename_template.__webpack_require__('style.css')calls orimport \"__RSPACK_ESM_CHUNK_<id>\"placeholders that point at chunks with no JS file.CssExtractRspackPluginhonorchunk.css_filename_template, sopreserveModules' per-chunk CSS filename overrides actually take effect.Test plan