fix: minify assets whose names carry a fragment - #747
Conversation
webpack keeps a `#fragment` in the asset name (`[hash][ext][query][fragment]` for asset modules, or a `#` in `output.filename`) and strips it only when writing. `test` already fell back to the bare name, but every built-in `filter`, the default `test`, the `.mjs`/`.cjs` detection and `extensionOf` accepted a query only, so such assets were skipped. Read the extension from before the first `?` or `#`. Claude-Session: https://claude.ai/code/session_01EpeANYvMKyZwSDTRVE5LoG
🦋 Changeset detectedLatest commit: 65b91f2 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #747 +/- ##
==========================================
+ Coverage 97.79% 97.86% +0.06%
==========================================
Files 5 5
Lines 1634 1637 +3
Branches 620 622 +2
==========================================
+ Hits 1598 1602 +4
+ Misses 36 35 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. WalkthroughThe plugin now recognizes file extensions followed by either a query or a fragment. Its extension helper removes both suffix types before deriving the extension. Extracted-comment filename parsing strips fragments while preserving query suffixes. Tests cover fragment-bearing assets and built-in minifier filters. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to Fragment-bearing assets are matched by their extensions, and extracted license filenames remain separate from JavaScript assets. No concrete merge risk remains in the reviewed changes. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
src/index.jsESLint failed to execute (timeout). test/extractComments-option.test.jsESLint skipped: the matched ESLint configuration already failed (timeout). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Remove the fragment before building the extracted-comments filename. · index.js:1113-1118
src/index.js:1113-1118
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove the fragment before building the extracted-comments filename.
When a newly eligible asset is named
main.js#abc, this code passesmain.js#abcasfilenameto anextractComments.filenamecallback. A callback that returns${fileData.filename}.LICENSE.txtproducesmain.js#abc.LICENSE.txt. Webpack removes fragments when writing assets, so that license asset and the JavaScript asset targetmain.json disk. Split the filename at the first?or#, and keep only a preceding query inquery. The documented callback usesfileData.filenamein this way. (webpack.js.org)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 7742aace-7b35-4361-b592-53cf92c14133
📒 Files selected for processing (7)
.changeset/fragment-asset-names.mdsrc/index.jssrc/utils.jstest/css-minify-option.test.jstest/fixtures/fragment-assets.jstest/helpers/readAsset.jstest/test-option.test.js
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
A function `extractComments.filename` built `main.js#abc.LICENSE.txt` from `fileData.filename`, which webpack writes onto `main.js` itself. Split the name at the first `?` or `#`; only a query is handed on as `query`. Claude-Session: https://claude.ai/code/session_01EpeANYvMKyZwSDTRVE5LoG
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 3097cb68-4826-4357-8288-1416bc663aca
📒 Files selected for processing (3)
src/index.jstest/extractComments-option.test.jstest/test-option.test.js
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.
1 participant
Summary
webpack keeps a
#fragmentin asset names (asset modules default to[hash][ext][query][fragment], andoutput.filenamemay contain#), but every built-infilter, the defaulttest, the.mjs/.cjsdetection andextensionOfaccepted only a?query, so e.g.main.js#abcpassedtestand was then skipped. The extension is now read from before the first?or#, which also stopsdata.txt#a.cssbeing claimed as CSS. Companion to webpack/webpack#22277.What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes —
test/test-option.test.js(JS/MJS/JSON builds with fragment names, plus every built-infilter) andtest/css-minify-option.test.js(CSS/HTML builds); both fail without the fix.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
Written with Claude Code: it reproduced the bug in a real webpack build, made the change and wrote the tests; the result was reviewed and directed by the author.
🤖 Generated with Claude Code
https://claude.ai/code/session_01EpeANYvMKyZwSDTRVE5LoG
Generated by Claude Code
Summary by CodeRabbit
.mjsand.cjsfiles.