test: add circular composes case for CSS modules#20929
Conversation
🦋 Changeset detectedLatest commit: 72fb606 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 |
|
This PR is packaged and the instant preview is available (d21b845). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@d21b845
yarn add -D webpack@https://pkg.pr.new/webpack@d21b845
pnpm add -D webpack@https://pkg.pr.new/webpack@d21b845 |
There was a problem hiding this comment.
Pull request overview
This PR tightens CSS Modules behavior to avoid creating duplicate module instances when the same .module.<ext> file is reached both directly (JS import) and indirectly via ICSS (composes from / :import), and adds a dedicated circular-composes fixture to pin self-referential behavior against css-loader.
Changes:
- Prevent duplicate CSS module instances by excluding
.module.<ext>from thecss-import-(local|global)-moduledefault rules (so.module.*continues to be classified by thecss/autorule). - Collapse
composes: <name> from "./self.module.css"insideself.module.cssto a self-reference (and treat exact same-name self-compose as a no-op). - Update/extend config-case tests and snapshots (including a new
composes-circular.module.cssfixture) and adjust css-modules config-case bundling expectations for deterministic IDs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/configCases/css/reexport/snapshots/ConfigTest.snap | Snapshot updates reflecting deduped CSS module output/class exports. |
| test/configCases/css/reexport/snapshots/ConfigCacheTest.snap | Cache snapshot updates reflecting deduped CSS module output/class exports. |
| test/configCases/css/css-modules/webpack.config.js | Narrows parser-option overrides to the entry stylesheet via a targeted rule. |
| test/configCases/css/css-modules/warnings.js | Updates expected warning ordering/occurrence for css-modules config cases. |
| test/configCases/css/css-modules/test.config.js | Makes bundle selection robust to deterministic numeric chunk prefixes. |
| test/configCases/css/css-modules/index.js | Reads the production CSS output using a directory scan for the deterministic chunk name. |
| test/configCases/css/css-loader/index.js | Adds the new circular composes fixture to the css-loader parity test list. |
| test/configCases/css/css-loader/composes-circular.module.css | New fixture covering self-referential composes from in the same file. |
| test/configCases/css/css-loader/snapshots/ConfigTest.snap | Snapshot updates + new snapshot entry for the circular composes fixture across export types. |
| test/configCases/css/css-loader/snapshots/ConfigCacheTest.snap | Cache snapshot updates + new snapshot entry for the circular composes fixture. |
| lib/css/CssParser.js | Adds self-reference detection for composes from and collapses same-file requests to self-references. |
| lib/config/defaults.js | Excludes .module.<ext> from `css-import-(local |
| .changeset/css-modules-dedup-icss-import.md | Changeset documenting the dedup fix and parser-option side effect. |
| .changeset/css-composes-circular-self-reference.md | Changeset documenting the self-referential composes from fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const moduleResourcePath = parseResource( | ||
| /** @type {string} */ (module.getResource()) | ||
| ).path; | ||
|
|
||
| /** | ||
| * Check whether a request points back to the current module | ||
| * (e.g. `composes: foo from "./self.module.css"` inside `self.module.css`). | ||
| * Only relative requests are checked — aliases / package / absolute requests | ||
| * fall through to the normal import path. | ||
| * @param {string} request request string from `from "<request>"` | ||
| * @returns {boolean} true if request resolves to the current module | ||
| */ | ||
| const isSelfReferenceRequest = (request) => { | ||
| if (!/^\.{1,2}\//.test(request)) return false; | ||
| if (!module.context) return false; | ||
| const requestPath = parseResource(request).path; | ||
| try { | ||
| return path.resolve(module.context, requestPath) === moduleResourcePath; | ||
| } catch (_err) { |
| let cssOutputFilename; | ||
| if (prod) { | ||
| const files = fs.readdirSync(__dirname); | ||
| cssOutputFilename = files.find(f => /^\d+\.bundle1\.css$/.test(f)); |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20929 +/- ##
==========================================
+ Coverage 91.35% 91.37% +0.02%
==========================================
Files 568 568
Lines 56466 56486 +20
Branches 14993 14999 +6
==========================================
+ Hits 51583 51614 +31
+ Misses 4883 4872 -11
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:
|
* `isSelfReferenceRequest`: also compare `query` and `fragment` from `parseResource`, since `NormalModuleFactory` keys modules on the full resource string. `composes: foo from "./self.module.css?bar"` from a parent without `?bar` is no longer collapsed to a self-reference, so resource-query variants stay distinct. * `test/configCases/css/css-modules/index.js`: throw an explicit error with the directory listing when no `*.bundle1.css` is found, instead of letting `path.join(undefined, ...)` blow up later with a less informative message. * `webpack.config.js` typing: extract the base rules array so the type checker can flow into the `[...baseRules, ...]` spread (`base.module` was typed as optional, breaking `lint:types-test`). https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid
| /** | ||
| * Check whether a request points back to the current module | ||
| * (e.g. `composes: foo from "./self.module.css"` inside `self.module.css`). | ||
| * Only relative requests are checked — aliases / package / absolute requests | ||
| * fall through to the normal import path. Requests with a `?query` or | ||
| * `#fragment` are only treated as self when the parent module's resource | ||
| * has the same query/fragment, since `NormalModuleFactory` keys modules | ||
| * on the full resource string. | ||
| * @param {string} request request string from `from "<request>"` |
Adds composes-circular.module.css covering `.local-name { composes:
local-name from "./composes-circular.module.css" }` (a class composing
itself from the same file) so the snapshot pins webpack's output for
self-referential composes against css-loader behavior.
https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid
A `composes: foo from "./self.module.css"` written inside
`self.module.css` previously created a CssIcssImportDependency that
factory-resolved into a second `css/module` instance of the same file
(the original was loaded as `css/auto`). The CSS chunk emitted both
copies with different localIdent hashes, and the JS export string
chained the duplicated class names.
CssParser now detects when a relative `from` request points back to the
current module and emits a SELF_REFERENCE export dependency instead of
an import + APPEND pair, matching css-loader. When the composed name
equals the local class name (e.g. `.foo { composes: foo from "./self.module.css" }`),
no extra dependency is emitted at all — it is a true no-op.
Aliased / package / absolute requests are intentionally untouched
because they require resolver work to compare safely.
https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid
The default rule
{ dependency: /css-import-(local|global)-module/, type: css/module }
forces any file pulled in by `composes: ... from "./x"` or
`:import("./x")` to type `css/module`. For plain `.css` files this is
needed so they're parsed as CSS modules. But for `.module.<ext>` files
the auto rule already classifies them as modules, and overriding the
type stamp to `css/module` makes `NormalModuleFactory` keep the
icss-imported instance separate from a sibling `css/auto` instance
loaded directly by JS — same resource, two module IDs, two `localIdent`
hashes, two copies of the file in the CSS chunk, and chained class
names in the JS export.
Add `exclude: /\.module\.\w+$/i` to both rules so `.module.css`,
`.module.less`, `.module.scss`, etc. fall back to whatever the file's
own rule assigns. The result is a single module instance, single hash,
single CSS block, css-loader-shaped exports.
Knock-on test fixture changes:
* `test/configCases/css/css-modules/index.js` and `test.config.js`:
the deterministic JS chunk filename pattern (`501.bundle1.js`,
`272.bundle3.js`, `244.bundle5.js`) was hardcoded — those module IDs
shifted because at-rule-value.module.css and friends now keep their
`css/auto` type. Resolved at runtime via a regex-scan of the output
directory so future shifts don't break the harness.
* `test/configCases/css/css-modules/warnings.js`: the per-config
warning order changed when at-rule-value.module.css went from
`css/module` to `css/auto`; the @value warnings now arrive before
the :global/:local warnings. Reordered to match.
* CSS chunk and JS export snapshots in `css-loader`, `css-modules`,
and `reexport`: the duplicated blocks / hash-suffixed class names
collapse to a single instance.
https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid
Parser options configured under `module.parser["css/auto"]` now also apply to `.module.<ext>` files reached via icss-import. Previously the forced `css/module` type silently overrode them. Users who relied on the old behavior need to mirror their `css/auto` parser options under `css/module`. https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid
The "should allow to disable" test set parser options under `module.parser["css/auto"]`, which now (post dedup fix) propagates to `@import`ed `.module.css` files. That correctly disabled custom-property hashing in `var-function.module.css` too — but it also wiped its `var(--foo from "./var-function-export.modules.css")` icss-imports, since `dashedIdents: false` short-circuits the parser before the import dependency is emitted. Move the option overrides to a rule-specific `parser` block keyed on `/style\.module\.css$/` so the disable applies only to the entry. The @imported `var-function.module.css` keeps default options, its custom properties stay hashed, and `var-function-export.modules.css` is loaded again. https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid
* `isSelfReferenceRequest`: also compare `query` and `fragment` from `parseResource`, since `NormalModuleFactory` keys modules on the full resource string. `composes: foo from "./self.module.css?bar"` from a parent without `?bar` is no longer collapsed to a self-reference, so resource-query variants stay distinct. * `test/configCases/css/css-modules/index.js`: throw an explicit error with the directory listing when no `*.bundle1.css` is found, instead of letting `path.join(undefined, ...)` blow up later with a less informative message. * `webpack.config.js` typing: extract the base rules array so the type checker can flow into the `[...baseRules, ...]` spread (`base.module` was typed as optional, breaking `lint:types-test`). https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid
`Defaults.unittest.js` pins the resolved default `module.rules` array. Adding `exclude: /\.module\.\w+$/i` to the `css-import-(local|global)-module` dependency rules in `defaults.js` changes that array, so the inline snapshots (and the surrounding context lines they reflow into) need to be regenerated. Ran `yarn test:unit -u`; only the two CSS-defaults snapshots updated. https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid
5047463 to
72fb606
Compare
Types CoverageCoverage after merging claude/add-circular-compose-test-syL5J into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Adds composes-circular.module.css covering
.local-name { composes: local-name from "./composes-circular.module.css" }(a class composingitself from the same file) so the snapshot pins webpack's output for
self-referential composes against css-loader behavior.
https://claude.ai/code/session_01LBzxuwEJhYcDUrD6xUQBid