fix: substitute [fullhash] in url() public paths for inlined CSS export types#21054
Merged
Conversation
…rt types Inlined CSS export types (style, text, css-style-sheet) leave the __WEBPACK_CSS_PUBLIC_PATH_FULL_HASH_ placeholder unsubstituted in url() because renderModule is called without the compilation hash.
🦋 Changeset detectedLatest commit: 4630f27 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 (66f71f8). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@66f71f8
yarn add -D webpack@https://pkg.pr.new/webpack@66f71f8
pnpm add -D webpack@https://pkg.pr.new/webpack@66f71f8 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a configCases test that reproduces a bug where [fullhash] in output.publicPath is not substituted in url() references for CSS modules using inlined exportTypes (text, css-style-sheet, style). The CSS placeholder __WEBPACK_CSS_PUBLIC_PATH_FULL_HASH_ remains in the rendered output because renderModule is invoked without the compilation hash for these export types. No production code fix is included.
Changes:
- New
test/configCases/css/export-type-fullhash-publicpathfixture withwebpack.config.jsenablingexperiments.css, three CSS files mapped to the three inlined exportTypes, and a shared image asset. index.jscontaining threeit(...)assertions that the rendered CSS for each exportType contains the substituted public path and not the raw placeholder.test.config.jsselectingbundle0.jsas the bundle under test.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/configCases/css/export-type-fullhash-publicpath/webpack.config.js | Configures [fullhash] publicPath and three exportType rules. |
| test/configCases/css/export-type-fullhash-publicpath/text.css | CSS fixture with url() for the text exportType. |
| test/configCases/css/export-type-fullhash-publicpath/sheet.css | CSS fixture with url() for the css-style-sheet exportType. |
| test/configCases/css/export-type-fullhash-publicpath/inject.css | CSS fixture with url() for the style exportType. |
| test/configCases/css/export-type-fullhash-publicpath/index.js | Assertions that the [fullhash] placeholder is resolved across all three exportTypes. |
| test/configCases/css/export-type-fullhash-publicpath/test.config.js | findBundle returning the JS bundle to execute. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+7
to
+28
| it("should substitute [fullhash] in url() for text exportType", () => { | ||
| expect(textCss).toContain("https://example.com/"); | ||
| expect(textCss).not.toContain(PLACEHOLDER); | ||
| }); | ||
|
|
||
| it("should substitute [fullhash] in url() for css-style-sheet exportType", () => { | ||
| expect(sheet).toBeInstanceOf(CSSStyleSheet); | ||
| const cssText = Array.from(sheet.cssRules) | ||
| .map(r => r.cssText) | ||
| .join("\n"); | ||
| expect(cssText).toContain("https://example.com/"); | ||
| expect(cssText).not.toContain(PLACEHOLDER); | ||
| }); | ||
|
|
||
| it("should substitute [fullhash] in url() for style exportType", () => { | ||
| if (typeof document === "undefined") return; | ||
| const styles = document.getElementsByTagName("style"); | ||
| const allStyleText = Array.from(styles) | ||
| .map(s => s.textContent) | ||
| .join("\n"); | ||
| expect(allStyleText).toContain("https://example.com/"); | ||
| expect(allStyleText).not.toContain(PLACEHOLDER); |
Member
Author
There was a problem hiding this comment.
The fix landed in this same PR (runtime __webpack_require__.h() substitution in CssGenerator._cssToJsLiteral), so these assertions now pass and the suite stays green. Title/description updated to reflect that the fix is included.
Generated by Claude Code
Codecov Report❌ Patch coverage is
❌ Your changes status has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes. Additional details and impacted files@@ Coverage Diff @@
## main #21054 +/- ##
==========================================
+ Coverage 89.69% 91.73% +2.04%
==========================================
Files 577 581 +4
Lines 60361 60658 +297
Branches 16321 16411 +90
==========================================
+ Hits 54139 55647 +1508
+ Misses 6222 5011 -1211
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:
|
…types Inlined CSS export types (style, text, css-style-sheet) embed CSS into JS at code generation time, before the compilation hash exists, so renderModule left the PUBLIC_PATH_FULL_HASH placeholder unresolved in url() public paths. Splice __webpack_require__.h() into the JS literal so the hash resolves at runtime, matching renderModule's build-time substitution for the link path.
Both renderModule (build-time hash) and CssGenerator._cssToJsLiteral (runtime __webpack_require__.h()) parsed the PUBLIC_PATH_FULL_HASH placeholder with duplicated logic. Move the scan into CssUrlDependency.walkFullHashPlaceholders and have both callers supply their own substitution via a callback.
| "webpack": patch | ||
| --- | ||
|
|
||
| Resolve `[fullhash]` in `url()` public paths for inlined CSS export types (`style`/`text`/`css-style-sheet`) at runtime. |
Member
Author
There was a problem hiding this comment.
Correct — the fix is included. Updated the PR title to fix: and rewrote the description to cover the fix plus the related refactor/perf cleanups.
Generated by Claude Code
renderModule re-materialized and re-scanned each module source for PUBLIC_PATH_AUTO / PUBLIC_PATH_FULL_HASH placeholders on every cache miss (distinct undoPath/hash/inheritance/runtime). The offsets are invariant for a given source, so compute them once into a weakly-keyed plan and reuse it, and skip the ReplaceSource wrapper entirely when a module has no placeholders.
HtmlGenerator._renderHtml ran the full dependency-template pass twice for extracted modules (once per HTML and JS source type), even though the passes differ only in the [webpack/auto] substitution. Cache the raw render per codeGeneration() call (keyed by its shared runtimeRequirements Set) and reuse it. Also memoize HtmlModulesPlugin.renderManifest's sentinel resolution and intermediate content hash per source, so a module landing in multiple chunks resolves its chunk-URL sentinels once instead of per (chunk, module).
Comment on lines
+1
to
+5
| --- | ||
| "webpack": patch | ||
| --- | ||
|
|
||
| Resolve `[fullhash]` in `url()` public paths for inlined CSS export types (`style`/`text`/`css-style-sheet`) at runtime. |
Member
Author
There was a problem hiding this comment.
Title and description updated to accurately describe the fix and the related caching/refactor work. Keeping it as one PR since the refactors (shared scanner, placeholder-offset cache, placeholder relocation) directly follow from the fix and share the same test coverage.
Generated by Claude Code
The PUBLIC_PATH_AUTO / PUBLIC_PATH_FULL_HASH placeholders and the walkFullHashPlaceholders scanner lived on CssUrlDependency, but they are a cross-cutting code-generation protocol: AssetGenerator and the HTML generator/plugin imported the CSS dependency only to reach them. Move them to lib/util/publicPathPlaceholder.js and import from there, so asset/css/html no longer couple to CssUrlDependency for this. Placeholder string values are unchanged.
Contributor
Types CoverageCoverage after merging claude/css-modules-export-hash-qFsY7 into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Summary
Inlined CSS export types (
style,text,css-style-sheet) embed CSS into JS at code-generation time, before the compilation hash exists, sorenderModuleleft the__WEBPACK_CSS_PUBLIC_PATH_FULL_HASH_placeholder unsubstituted inurl()public paths that use[fullhash]/[hash]— shipping a broken URL. Thelinktype was unaffected (it substitutes the real hash at render time).The fix splices
__webpack_require__.h()into the JS string literal so the hash resolves at runtime, mirroringrenderModule's build-time substitution for thelinkpath. The PR also includes follow-on cleanups: a sharedwalkFullHashPlaceholdersscanner, a per-source placeholder-offset cache inCssModulesPlugin(avoids re-materializing/re-scanning module sources), analogous dedup of redundant HTML render work, and moving the public-path placeholder constants tolib/util/publicPathPlaceholder.js(they were a cross-cutting protocol parked onCssUrlDependency). Placeholder string values are unchanged.What kind of change does this PR introduce?
fix (with related refactor/perf cleanups)
Did you add tests for your changes?
Yes —
test/configCases/css/export-type-fullhash-publicpath/covers all three inlined export types across[fullhash]and[fullhash:8]public paths. Existing CSS/HTML/asset suites (snapshots byte-identical) cover the refactors.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
AI (Claude) was used to investigate the bug, write the reproduction test, implement the fix, and perform the follow-on refactor/perf work; all changes were reviewed and verified against the test suites.