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

Play better with postcss-modules #214

Closed
longlho opened this issue Jun 5, 2016 · 15 comments
Closed

Play better with postcss-modules #214

longlho opened this issue Jun 5, 2016 · 15 comments

Comments

@longlho
Copy link

longlho commented Jun 5, 2016

If I stack those 2 together

require('postcss-import')(),
require('postcss-modules')()

variables.css

:root {
  --white: white;
  --black: black;
}

foo.css:

@import 'variables.css';
.foo {
  color: var(--white);
}

bar.css

@import 'variables.css';
.bar {
  composes: foo from 'foo.css';
  background-color: var(--black);
}

postcss-modules's composes also inlines the file foo, so now I got an @import that's not at the top of the file, which violates the spec. The (broken) result is something like:

@import 'variables.css';
.foo {
  color: var(--white);
}
@import 'variables.css';
.bar {
  background-color: var(--black);
}

Any suggestion on how to address this issue? I'm not using webpack (just pure PostCSS). Do I have to write a plugin to dedupe & move imports up top?

Thanks!

@MoOx
Copy link
Contributor

MoOx commented Jun 6, 2016

If postcss-import is ran first, composes should just be applied after so not sure neither what is the issue since you are not using standard syntax.

@longlho
Copy link
Author

longlho commented Jun 6, 2016

@MoOx tks for the reply. What do u mean by not standard syntax 😢 ?

@MoOx
Copy link
Contributor

MoOx commented Jun 6, 2016

composes: foo from 'foo.css'; is not valid CSS.

@longlho
Copy link
Author

longlho commented Jun 6, 2016

Yup its a CSS Modules concept, thus the postcss-modules plugin :(
On Sun, Jun 5, 2016 at 11:31 PM Maxime Thirouin notifications@github.com
wrote:

composes: foo from 'foo.css'; is not valid CSS.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#214 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAMGb1m2gC4ciiZUGz5wo-vB9_IOF1Noks5qI5R7gaJpZM4IuRvP
.

@RyanZim
Copy link
Collaborator

RyanZim commented Nov 3, 2016

Related: #227.

In the future we should probably add an option to turn off forcing @imports to be at the top.

@RyanZim RyanZim changed the title usage with postcss-modules Play better with postcss-modules Nov 3, 2016
@leebenson
Copy link

leebenson commented Apr 23, 2017

@RyanZim - did the option to turn off top-level @import enforcement get added? Sorry if this was answered elsewhere - tried looking through old threads, but couldn't see anything obvious.

@RyanZim
Copy link
Collaborator

RyanZim commented Apr 24, 2017

@leebenson Actually, I don't think adding such an option is possible; since postcss-import supports media query imports, all imports have to be at the top. Closing since I wasn't aware of this limitation when I wrote the above comment.

@RyanZim RyanZim closed this as completed Apr 24, 2017
@leebenson
Copy link

Ah, thanks @RyanZim, that makes sense. I think a few devs are getting hung-up on how to do 'global' imports of third-party scripts, inside a :global block that postcss-modules or css-loader uses, like this:

:global {
    @import 'someExternalStyle.css';
}

Since @import automatically puts that at the top-level, it side-steps the :global nesting and dumps it out as a localised module.

In my own starter kit, I have a separate config for .global.css file loading that avoids localising the classes. Was trying to get consistent behaviour inside a :global block, but I can see now that's not viable. Thanks @RyanZim.

@michael-ciniawsky
Copy link

michael-ciniawsky commented Apr 24, 2017

@leebenson postcss-partial-import/postcss-nested-import, but it's not recommended to use @import via postcss-import at all with css-loader, this could lead to duplicates within the CSS modules, which could have been avoided by letting css-loader handle the @import => require(path/to/import) instead. If you have Global CSS (libs, etc..) just use a CDN or add an extra rule, the :global should be used only if you want to globalize a component style for some reason. (rare case)

@leebenson
Copy link

Thanks @michael-ciniawsky. Your advice is actually exactly how I have it set-up currently, I was just curious whether it was possible to mimic the behaviour within a :global block-- I see now that it's not a 1:1 alternative.

@michael-ciniawsky
Copy link

Maybe possible 😛 (when using CSSModules with gulp, grunt), but imho should be consider bad practice in general, this will become hard to maintain quickly and doesn't have any real benefit at all, more the opposite :)

@michael-ciniawsky
Copy link

composes: a, b, c from 'file.css' for :local (by default) and rarely :global when needed, the rest is something we even intent to get rid of soon, since it always caused more trouble then good and isn't necessary :D

@eriklharper
Copy link

The primary benefit I see with being able to define imports inside a :global {} block is for the purpose of namespacing third party class names, which prevents those classnames from truly polluting the global CSS, since they're scoped to whatever containing class you import them inside, which in my case is a css modules hashed classname.

Yes, it doesn't take into consideration duplicates, but it does enable true encapsulation of styles to the specific component that needs those third party styles. Examples include leaflet, react-widgets and draft-js, robust third party javascript libraries for building maps, DateTime pickers and rich text editors that need a decent amount of boilerplate CSS to get them rendering properly, and which can be overridden with your consuming app's specific styles.

Its easy to say "don't do globals at all" but the reality is that we still need to be able to do globals if we want to consume web components authored by other developers. Personally I feel like putting my global imports directly in my document (as opposed to importing them in situ at the component level) defeats the purpose of modularizing CSS to begin with.

@leebenson
Copy link

leebenson commented Apr 24, 2017

@eriklharper - just to clarify on this thread, the way I was able to 'fix' it in my case was to make globals the work of a separate loader, triggered by the extension .global.css.

This avoids having to rely on :global blocks to import stuff, which turned out not to be fool-proof anyway (especially if you're using SASS with @at-rule, or have multiple classnames separated by commas that don't all need a :global prefix which can easily mangle the 'nesting' that something like postcss-nested needs to do to prefix each and every rule with :global).

Instead, by making .global.css use { modules: false } in css-loader, you can rest assured that any and all imports - and any/all classnames - will wind up in your final CSS bundle with everything perfectly intact.

Then your 'plain' .css files can continue to spit out localised versions, under a separate loader that uses { modules: true }

So, using :global winds up being - like @michael-ciniawsky said - for those rare instances when you want to preserve an 'inline' classname rather than a full import.

@dehuszar
Copy link

dehuszar commented Jan 7, 2018

I'm a little confused as to the resolution here.

Will one ever be able to @import file.css from inside an AtRule again? Media Query scopes appear to be supported, but current versions of this plugin no longer function with my plugin (postcss-reference) which processes statements in a custom AtRule, forcing me to encourage users to use postcss-nested-import or stay at postcss-import v7 (after which things break).

I get the desire to adhere to the spec, but this is a CSS processing toolchain, most @import statements will have been stripped out during a compilation process, and so the browser will never see @imports not at the top.

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

No branches or pull requests

7 participants