Skip to content
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

Plugins run twice on css modules #70

Closed
pascalduez opened this issue Feb 10, 2018 · 14 comments · Fixed by #249
Closed

Plugins run twice on css modules #70

pascalduez opened this issue Feb 10, 2018 · 14 comments · Fixed by #249
Labels

Comments

@pascalduez
Copy link
Contributor

Hi,

I'm still investigating the issue but wanted some feedback:

postcss({
  modules: true,
  extract: true,
  plugins: [
    require('postcss-custom-properties')({ preserve: true })
  ],
})

Result:

.Button_primary__1gH9y {
  color: green;
  color: green;
  color: var(--color-primary);
}

I did some logging and the postcss-custom-properties is run twice on the same css module, hence the duplicate fallback.

1st

.primary {
  color: var(--color-primary);
}

2nd

.Button_primary__1gH9y {
  color: green;
  color: var(--color-primary);
}
@pascalduez
Copy link
Contributor Author

Ok, narrowed to madyankin/postcss-modules/issues/70

@pascalduez
Copy link
Contributor Author

Also using CSS modules composition could result in a bunch of duplicate rules.

/* Shared.css */
.test {
  content: 'test';
}

/* Foo.css */
.foo {
  composes: test from './Shared.css';
}

/* Bar.css */
.bar {
  composes: test from './Shared.css';
}

Result:

.Shared-test {
  content: 'test';
}

.Foo-foo {}

.Shared-test {
  content: 'test';
}

.Bar-bar {}

@peter-mouland
Copy link

I ran into this issue today, was there a fix for this?

@n370
Copy link
Contributor

n370 commented Mar 9, 2020

I found that changing https://github.com/egoist/rollup-plugin-postcss/blob/master/src/postcss-loader.js#L75 from push to unshift fixes the issue.

n370 added a commit to n370/rollup-plugin-postcss that referenced this issue Mar 9, 2020
Fixes egoist#70 

If the postcss-modules plugin isn't placed first we end up with non-modularized rules along with modularized rules. This situation thus leads to heavier bundles due to code duplication that end up dead and, when the extract option is set to false, it can cause conflicts with other non-modularized (global) rules present in the bundle consumer code.
himself65 pushed a commit that referenced this issue Mar 9, 2020
Fixes #70 

If the postcss-modules plugin isn't placed first we end up with non-modularized rules along with modularized rules. This situation thus leads to heavier bundles due to code duplication that end up dead and, when the extract option is set to false, it can cause conflicts with other non-modularized (global) rules present in the bundle consumer code.
@Anidetrix
Copy link

Anidetrix commented Mar 9, 2020

@n370 @himself65 this change breaks such plugins as postcss-import, cause imported classes won't be modularized

@n370
Copy link
Contributor

n370 commented Mar 9, 2020

@Anidetrix yeah, you're right. We need to find a better solution. One easy solution I can think of is to maybe add a hardcoded list of plugin names that need to be loaded before postcss-modules and then split the user provided list of plugins into a list of plugins to be loaded before postcss-modules and a list to be loaded after. But really that's my naive take since I have just a small experience with the codebase.

@Anidetrix
Copy link

Anidetrix commented Mar 9, 2020

@n370 This is due to the fact that postcss-modules calls PostCSS inside itself, thus causing the doubling. The proper solution is to adapt separate PostCSS plugins for modules as it is done in webpack's css-loader.

@n370
Copy link
Contributor

n370 commented Mar 9, 2020

@Anidetrix I haven't observed that behavior in my use case but that might be the case. Anyways, I was thinking about why one would want to use the postcss-import plugin when they already have sass, less and stylus built in.

@Anidetrix
Copy link

@n370 probably with something like SugarSS

@n370
Copy link
Contributor

n370 commented Mar 9, 2020

got you!

@egoist
Copy link
Owner

egoist commented Mar 10, 2020

🎉 This issue has been resolved in version 2.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@pascalduez
Copy link
Contributor Author

The issue is still alive: rollup-plugin-postcss@3.1.4

@danny-andrews
Copy link

danny-andrews commented Nov 14, 2020

Can confirm, I still see this issue on 3.1.8.

When I use composes: classname from "./some-module.js", I end up with that rule definition being duplicated in the css output.

@danny-andrews
Copy link

This particular issue is a dupe of #26.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants