-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CSS Modules - composed styles are duplicated #7504
Comments
I'm not particularly familiar with CSS Modules, but it works fine when you change their import order. import originalStyles from './styles/original.module.css';
import smallStyles from './styles/small.module.css'; // import later for higher priority |
In this small example it does have a workaround. But when I'm composing classes across many different css modules in my actual project, I have no way of controlling the import order myself. That's entirely in vite territory. |
@desmond-tuiyot You are right, I tested it in the webpack project with no problems |
I also have the same problem |
css-modules indicates that the order is not granted when composing from multiple files. So maybe the best practice is to split the properties into different files. https://github.com/css-modules/css-modules#composing-from-other-files
|
I think that that disclaimer applies to situations when you're composing multiple classes into one style rule. For example a {
composes: b from './b.module.css';
composes: c from './c.module.css';
}
} My case is different. And also in my actual project, I'm not importing 2 different css module into one file. This example is set up that way for simplicity |
I think this problem cannot be solved with Vite due to the architecture.
To dedupe One way it may solve this is to rewrite |
In #5185, I reported an ordering issue that @sapphi-red rightfully identified as a consequence of this duplication issue. In my case, I was looking at a compiled CSS output, which did not have duplicated styles. I assume this is because If so, then my ordering issue was most likely caused by If this alternative deduplication behaviour cannot be implemented in |
Ah, I noticed there's two issues around this one. The first one is the duplication issue. This cannot be solved as I described #7504 (comment). The second one is the ordering issue. I noticed this can be solved by leaving the duplication issue. By including the output filename in hash calculation (or changing the meaning of
Yes, this should be because cssnano (or other css minifier) is removing the duplicated ones.
This is not correct and will cause other issues (and maybe this ordering issue will also happen). |
I stumbled upon both these issues as soon as I implemented second route/page of the first app where I use these features. I am very surprised this issue isn't reported way more often. For posterity: the solution to ordering issue is to change architecture of your css modules to never overwrite css properties declared on the base class. You should only build upon base class with new/unique properties but never overwrite. For complex cases you will have a tree of classes where only leaf nodes are actually used as classNames for HTML elements (you will also have plenty of mixins). Of course this only applies if you compose from other files - in the context of one file you can do whatever you want. It feels to me that this is just a trait of CSS Modules in their current form and can not be fixed. To depend on ordering of CSS declarations feels fragile and prone to being mishandled by post processing plugins. |
Is this hard to do with Vite plugins architecture? I think Vite can be very awesome in this field. |
I think this issue should be fixable without processing all CSS files at once. The problem here is that when a CSS Module is imported and it has a I would like to see this fixed since I'm moving to Storybook 7 with Vite. This issue breaks our current code architecture. Many of our CSS Modules have zero specificity selectors in order to have no issues on consumer side of component library, avoiding potential issues with CSS file loading order. The existence of this bug forces me to increase specificity selectively based on whether a CSS Module uses I think one of the reasons this issue does not come up that often is that you will mostly notice this issue if you are a veteran CSS Module user, or if you're moving existing code to use Vite. If you are less experienced with CSS Modules and/or are working on new code then you probably think CSS Modules is limited, and continue on. For some humor to the ending, I will not be using Webpack for Storybook 7 because it will error with "out of memory" and debugging that would be worse than this issue in Vite... 😅 |
I've implemented a fix via https://github.com/privatenumber/vite-css-modules (which is getting integrated into Vite core via #16018). Testing and feedback will be appreciated! |
This is awesome to get this fixed. If it's helpful to anyone else, I've previously overcome any ordering issues by making sure anything that is shared/composed is put in a css @layer{} so that you can then control how it will be stacked in the cascade and then get rid of any duplicates with css nano. (If you have the luxury of only supporting browsers that support css layers) |
Since browser support can be an issue with /* utils.module.css (never imported directly) */
.baseBtn {
color: blue;
}
/* OtherComponent.module.css (imported from `OtherComponent.jsx`) */
.otherBtn {
composes: baseBtn from './utils.module.css';
}
/* ThisComponent.module.css (imported from `ThisComponent.jsx`) */
.thisBtn {
/*composes: otherBtn from './OtherComponent.module.css';*/ /* FORBIDDEN */
composes: baseBtn from './utils.module.css'; /* ALLOWED */
color: red;
} With @privatenumber's plugin, this guideline seems to remove any remaining ordering issue. If multiple components import the same CSS module, you still get duplicated styles in development, but if multiple components' modules compose from the same utility module, this utility modules' styles are no longer duplicated and now always come before the components' modules. The guideline could be enforced by a linting rule to avoid mistakes. It could be as simple as checking that the filename of the composed module starts with a lower case letter (since other linting rules enforce that React component files start with an uppercase letter). |
Describe the bug
Importing a css module that's been composed ends up duplicating styles.
In the reproduction linked below, our
App.tsx
has 2 div elements with texts that have different styles. The styles are under thestyles
folder. The first text is styled usingoriginalText
style fromoriginal.module.css
, and should have a 32px font size. The second one should have a 14px font size, since it's styled usingsmallText
fromsmall.module.css
, which composesoriginalText
and then overrides the font size. However, it doesn't work as expected.The
originalText
style fromoriginal.module.css
is imported first, thensmallText
style is imported, then theoriginalText
style is imported once again, overriding the font size provided bysmallText
.You can see this in action if you inspect the div element in dev tools and check out the styles applied.
Reproduction
https://codesandbox.io/s/vite-react-ts-forked-dy91of?file=/src/App.tsx
System Info
Used Package Manager
npm
Logs
No response
Validations
The text was updated successfully, but these errors were encountered: