fix: fix HMR for concatenated CSS modules with style exportType#20911
Conversation
🦋 Changeset detectedLatest commit: deb6003 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 (a5521a0). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@a5521a0
yarn add -D webpack@https://pkg.pr.new/webpack@a5521a0
pnpm add -D webpack@https://pkg.pr.new/webpack@a5521a0 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #20911 +/- ##
==========================================
+ Coverage 91.38% 91.39% +0.01%
==========================================
Files 570 570
Lines 57178 57190 +12
Branches 15243 15249 +6
==========================================
+ Hits 52250 52269 +19
+ Misses 4928 4921 -7
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:
|
63a0189 to
66df144
Compare
Merging this PR will not alter performance
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
66df144 to
373a9dd
Compare
There was a problem hiding this comment.
To be honestly reloading is a good solution here, concatenated CSS module technically can be concatenated with other JS module which make side effect and don't have HMR logic, although technically it is a rare case because we filter, I think no one really use concatenation with HMR
|
@alexander-akait Yeah, concatenation + HMR is a rare combination. Our fix only manages <style> element lifecycle, it doesn't change JS HMR behavior. Also, this fix is relatively small, and with a guard like |
|
@xiaoxiaojx Yeah, I am fine with it, let's merge it a little bit later, want to fix more small bugs and write tests cases in generator and then we will rebase and merge 👍 |
ba0ce03 to
d663d39
Compare
| // CSS modules need IDs even when not in chunks, for generating CSS class names(i.e. [id]-[local]) | ||
| /** @type {BuildMeta} */ (module.buildMeta).isCssModule) | ||
| /** @type {BuildMeta} */ (module.buildMeta).isCssModule || | ||
| /** @type {BuildMeta} */ (module.buildMeta).needIdInConcatenation) |
There was a problem hiding this comment.
@xiaoxiaojx Let's set needIdInConcatenation when it is a css module, I think checking needIdInConcatenation should be enough and less checks
There was a problem hiding this comment.
There’s a subtle difference between isCssModule and needIdInConcatenation here.
isCssModule means the module always needs an id, regardless of the scenario.
needIdInConcatenation only means the id is required during concatenation.
For example, during the HMR phase, with isCssModule we don’t need to traverse child modules, but for needIdInConcatenation we still do.
- Use stable per-module identifiers ([moduleId, style] tuples) instead of positional indices for injected style elements in concatenated CSS modules - Track inner module IDs of ConcatenatedModules in HMR records so removed sub-modules appear in hotUpdateMainJson.m - Add buildMeta.needIdWithoutChunk flag to ensure style-type CSS modules retain module IDs when disconnected from chunks during concatenation
d663d39 to
deb6003
Compare
Summary
What kind of change does this PR introduce?
fix
Did you add tests for your changes?
Yes
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?
Nothing