preserve @charset in CSS modules with exportType "text"#20912
Merged
Conversation
The charset extracted by the parser was only re-added for "css-style-sheet" exports, so "text" exports lost it. Prepend the charset to the exported text and strip leading @charset from concatenated text imports to avoid duplicate directives in the resulting string.
Move the @charset handling fully to the build step. The previous commit added a runtime regex on each concatenated text import to dedupe nested @charset directives; instead, just bake the @charset into the wrapper at build time. Nested text-from-text imports may produce multiple @charset lines in the output, which is still valid CSS (parsers honor only the first one at byte 0).
Replace the runtime regex with a build-time-computed `.slice(N)` on each text-from-text import expression. The imported module's @charset value is known at build time, so the prefix length `'@charset "X";\n'.length` is emitted as a literal. This produces a single @charset directive at byte 0 of the final output without any regex or extra runtime helper.
🦋 Changeset detectedLatest commit: 03ea920 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
|
This PR is packaged and the instant preview is available (b34c597). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@b34c597
yarn add -D webpack@https://pkg.pr.new/webpack@b34c597
pnpm add -D webpack@https://pkg.pr.new/webpack@b34c597 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20912 +/- ##
==========================================
- Coverage 91.37% 91.35% -0.02%
==========================================
Files 563 563
Lines 55818 55855 +37
Branches 14786 14810 +24
==========================================
+ Hits 51002 51026 +24
- Misses 4816 4829 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates webpack’s CSS generator so exportType: "text" preserves @charset, matching the existing behavior for css-style-sheet exports, and adjusts composed text imports to avoid duplicate leading charset directives.
Changes:
- Prepend
@charsetfor CSS modules exported as text. - Strip a leading
@charsetfrom directly imported text exports when composing a text export. - Update charset snapshots and add a patch changeset entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/css/CssGenerator.js |
Adds charset preservation for text exports and trims imported text prefixes during concatenation. |
test/configCases/css/charset/__snapshots__/ConfigTest.snap |
Updates expected text-export snapshots to include the preserved charset. |
test/configCases/css/charset/__snapshots__/ConfigCacheTest.snap |
Mirrors the snapshot updates for cached config test cases. |
.changeset/css-modules-text-export-charset.md |
Documents the patch-level behavior change for text exports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+173
to
+181
| const depCharset = | ||
| ownCharset !== undefined && | ||
| getDefaultExport && | ||
| depModule.exportType === "text" && | ||
| depModule.buildInfo | ||
| ? /** @type {BuildInfo} */ (depModule.buildInfo).charset | ||
| : undefined; | ||
| const charsetPrefixLen = | ||
| depCharset !== undefined ? `@charset "${depCharset}";\n`.length : 0; |
Member
Author
There was a problem hiding this comment.
Good catch — fixed in 03ea920. The slice/prepend now use an effective charset that walks through text imports when a module has no local @charset, and the test fixtures (leaf-with-charset.text.css → inherit-charset.text.css → styles-7.text.css) cover both the inherited case directly and inherited-then-imported-by-charset-owner case.
Generated by Claude Code
A text module can have no local @charset but still expose a default that starts with @charset because a transitively-imported text module declared one. Previously the slice computation only used the import's own buildInfo.charset, missing the inherited case and leaving a duplicate @charset in the middle of the consumer's text. Walk through text imports to compute the effective charset and use it for both the wrapper prepend and the build-time slice length. Add fixtures covering the transitive case.
Contributor
Types CoverageCoverage after merging claude/fix-css-modules-charset-BXpYK into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Member
Author
|
@copilot review |
Here's a summary of the current state of the PR: What the PR does
Test coverage (all passing — 8 tests, 14 snapshots)
No issues found. The implementation correctly handles direct, inherited, and mixed (owner + inheritor) charset scenarios. The cycle guard ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The charset extracted by the parser was only re-added for "css-style-sheet"
exports, so "text" exports lost it. Prepend the charset to the exported
text and strip leading @charset from concatenated text imports to avoid
duplicate directives in the resulting string.