vite icon indicating copy to clipboard operation
vite copied to clipboard

CSS Modules - composed styles are duplicated

Open desmond-tuiyot opened this issue 3 years ago • 18 comments

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 the styles folder. The first text is styled using originalText style from original.module.css, and should have a 32px font size. The second one should have a 14px font size, since it's styled using smallText from small.module.css, which composes originalText and then overrides the font size. However, it doesn't work as expected.

The originalText style from original.module.css is imported first, then smallText style is imported, then the originalText style is imported once again, overriding the font size provided by smallText.

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

System:
    OS: macOS 12.2.1
    CPU: (8) arm64 Apple M1
    Memory: 127.45 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.2 - /usr/local/bin/node
    Yarn: 1.22.17 - /opt/homebrew/bin/yarn
    npm: 8.1.2 - /usr/local/bin/npm
  Browsers:
    Brave Browser: 99.1.36.117
    Chrome: 99.0.4844.83
    Firefox: 97.0.1
    Safari: 15.3
  npmPackages:
    @vitejs/plugin-react: ^1.2.0 => 1.2.0 
    vite: ^2.8.6 => 2.8.6

Used Package Manager

npm

Logs

No response

Validations

desmond-tuiyot avatar Mar 28 '22 21:03 desmond-tuiyot

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

ygj6 avatar Mar 30 '22 12:03 ygj6

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 avatar Mar 30 '22 13:03 desmond-tuiyot

@desmond-tuiyot You are right, I tested it in the webpack project with no problems

ygj6 avatar Mar 31 '22 03:03 ygj6

I also have the same problem

xiaoxian521 avatar Apr 01 '22 08:04 xiaoxian521

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

Note that when composing multiple classes from different files the order of appliance is undefined. Make sure to not define different values for the same property in multiple class names from different files when they are composed in a single class.

fi3ework avatar Apr 06 '22 06:04 fi3ework

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

desmond-tuiyot avatar Apr 06 '22 06:04 desmond-tuiyot

I think this problem cannot be solved with Vite due to the architecture. Vite processes each css import individually.

  • original.module.css will include .originalText
  • small.module.css will include .originalText and .smallText

To dedupe .originalText, it will need to process all css files at once because it needs to know depency graph.

One way it may solve this is to rewrite composes into a import with JS. But I feel this is no longer "CSSModules".

sapphi-red avatar Apr 22 '22 16:04 sapphi-red

FYI, https://github.com/madyankin/postcss-modules/issues/32

ygj6 avatar Apr 27 '22 06:04 ygj6

In https://github.com/vitejs/vite/issues/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 cssnano removes the duplicated styles at build time?

If so, then my ordering issue was most likely caused by cssnano keeping only the last occurrence of a duplicated style. Am I wrong in thinking that by keeping only the first occurrence, the issue would be resolved for the most part? This solution probably has edge cases, but it could be a decent way forward.

If this alternative deduplication behaviour cannot be implemented in cssnano, could it be done by a dedicated Vite plugin? Maybe this plugin could even work in development to remove duplicated styles dynamically?

axelboc avatar Aug 10 '22 11:08 axelboc

Ah, I noticed there's two issues around this one.

The first one is the duplication issue. This cannot be solved as I described https://github.com/vitejs/vite/issues/7504#issuecomment-1106675158.

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 filename argument of generateScopedName), I guess this can be solved. But this will be a breaking change so it won't be possible to include until next major.

In my case, I was looking at a compiled CSS output, which did not have duplicated styles. I assume this is because cssnano removes the duplicated styles at build time?

Yes, this should be because cssnano (or other css minifier) is removing the duplicated ones.

Am I wrong in thinking that by keeping only the first occurrence, the issue would be resolved for the most part?

This is not correct and will cause other issues (and maybe this ordering issue will also happen).

sapphi-red avatar Aug 10 '22 12:08 sapphi-red

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.

hyperscientist avatar Sep 23 '22 15:09 hyperscientist

To dedupe .originalText, it will need to process all css files at once because it needs to know depency graph.

Is this hard to do with Vite plugins architecture? I think Vite can be very awesome in this field.

Tal500 avatar Oct 10 '22 20:10 Tal500

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 composes, Vite outputs all CSS to a single style element. This is the bug. Vite should output multiple style elements, one for each module. And, if a module of the same name has already been injected then simply do not output it again. After this there would be no more duplication of selectors. (Of course I'm no expert in Vite internals, behavior, nor limitations.)

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 composes or not. This will also force me to release a new major version of the library as increasing specificity is a potential breaking change. Also, working around this Vite composes issue will be extra mental load that nobody should be dealing with.

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... 😅

Merri avatar Jul 31 '23 10:07 Merri

I've implemented a fix via https://github.com/privatenumber/vite-css-modules (which is getting integrated into Vite core via https://github.com/vitejs/vite/pull/16018).

Testing and feedback will be appreciated!

privatenumber avatar Feb 29 '24 00:02 privatenumber

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)

cssandstuff avatar Mar 18 '24 00:03 cssandstuff

Since browser support can be an issue with @layer, I've personally started documenting/implementing a dev guideline that forbids composing from another (React) component's CSS Module. If composition is needed, the base styles have to be moved to a "utility" CSS Module that must never be imported into a component:

/* 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).

axelboc avatar Mar 18 '24 13:03 axelboc

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!

@privatenumber Please have a look at this fork of the OP's repro using your plugin and the newest version of Vite: https://codesandbox.io/p/sandbox/vite-react-ts-forked-dy91of. The style overwriting is still there unfortunately :(

aweebit avatar Jun 12 '25 02:06 aweebit

I have to say this issue is the one thing that made me switch to Rsbuild in my personal project. I didn't realize Vite's style duplication was the issue because I was already using @privatenumber's plugin, so I ended up thinking this was just some inherent flaw of CSS Modules that couldn't be fixed. I then searched half the internet for a decent CSS-in-JS solution, but couldn't find any that would meet my needs: most of the ones that support static CSS file extraction are built on the concept of atomic styles that I really don't like, and Linaria, while being probably the only one that isn't based on atomicity, unfortunately doesn't have style composition at all. I learnt way more about CSS-in-JS than I had ever planned to (could really write an entire paper on the topic at this point), but in the end, the issue turned out to be caused by Vite. This duplication makes CSS Modules pretty much unusable, and that's a shame considering how even after thorough research I couldn't find any CSS-in-JS alternative that would be as performant and ergonomic as CSS Modules are. I really hope this gets fixed soon. Rsbuild somehow doesn't have this problem, so maybe you could look at what their approach to style ordering is for inspiration.

aweebit avatar Jun 12 '25 03:06 aweebit