postcss/postcss-import

Issue with `@layer`s

argyleink opened this issue ยท 12 comments

given

@layer foo {
  :root {
    color: red;
  }
}
@import "icons.css" layer(base);

output is missing the contents of icons.css:

@layer foo {
  :root {
    color: red;
  }
}

if i remove the layer foo, the output is:

@layer base {
.filled-icon {
  color: green;
}
}

Seems like there's a bug when there are inline layers already present in the file?

@import statements must come first in CSS files; you should have gotten a warning about this; what postcss runner are you using?

Confirmed that a warning is emitted by the plugin for this source code.

unexpected warning: "@import must precede all other statements (besides @charset or empty @layer)"

statements must come first

totally, but, that's what i'm inlining for. once inlined, there's nothing to warn about. plus, why warn and remove the statement?

what postcss runner?

npm postcss in deno


you may be wondering why i have that line of code in my styles? another plugin is importing files from a url into a layer in a transform step before this plugin. i think it'd be best if this plugin didnt care where the import was and instead just replaced it where it found it? or, add the ability to import remote URLs so this plugin can own all the imports and not find itself 2nd in the build step.

thanks y'all for checking this out tho so quickly, much appreciated!

npm postcss in deno

Yeah, you'll want to modify your code to check for result.warnings to get the message about problems like this.

This plugin will always follow the CSS spec; when warnings are being omitted because there was invalid input, I don't think it particularly matters what the output is. I'm not sure that a fully understand what you're trying to do, but I see no reason that this plugin needs to change its behavior here.

really, nothing to change? it's removing something entirely and using a warning as a way to not do something? i agree this plugin should follow specs, but not that it should be opinionated in this way. i was assuming the use case i had made it clear of the what and why..

i see the other bug was resolved, so maybe i can put this plugin into the transform pipeline in a way that can work around the requirement/warning. but ultimately, i'd prefer this plugin to inline imports, wherever they are. it's fine if it warns about what it did, but my expectation was an import just get inlined. the final output is spec compliant.

but my expectation was an import just get inlined. the final output is spec compliant.

I partly agree with @RyanZim here.
We follow the same principles in all plugins under postcss-preset-env.

It should be possible to remove a plugin while still having the same outcome by letting browsers take over and handle the feature.

This isn't possible when the input is :

@layer foo {
  :root {
    color: red;
  }
}
@import "icons.css" layer(base);

Inlining that @import would conflict with what a browser would do.

keeping in mind that this plugin doesn't know about other plugins and needs to assume it is the only actor handling @import.


I can however see the benefits of emitting a warning while preserving the invalid @import statement.

That is a seemingly harmless change : #514

But as I said in the pull request I don't have a strong preference, the outcome is invalid CSS either way.


I am curious however why another plugin is handling @import and why it is only doing this for some rules.

Seems like it would be better if only one plugin was doing this.
Maybe the solution isn't to change this plugin but the other one?

Seems like it would be better if only one plugin was doing this.

totally, i agree.
right now i have 3 plugins for imports and i'd love to just have 1...

  1. glob import
  2. this import
  3. remote url import

if this plugin inlined remote urls then i would have never found this bug. the bug is because the remote url script ran before this one, therefore inlining a 3rd party import into a layer in front of my app imports. then since this plugin is opinionated about it's position in the plugin stack, it is unhappy with the intermediate state of the stylesheet.

i feel like the warning should be a lint warning, then it's a warning in at a better time/moment in authoring, not a warning produced by this plugin. especially since the warning is about a temporary state of the stylesheet.

if this plugin inlined remote urls then i would have never found this bug. the bug is because the remote url script ran before this one, therefore inlining a 3rd party import into a layer in front of my app imports. then since this plugin is opinionated about it's position in the plugin stack, it is unhappy with the intermediate state of the stylesheet.

Need to think about this ๐Ÿค”
It can easily be the other way around depending on plugin order and source order.

@import "https://example.com/styles.css"
@import "my-styles.css"

vs.

@import "my-styles.css"
@import "https://example.com/styles.css"

totally, @layer is actually here to rescue this but only if you use them..

also, when you flip it the other way around, this plugin could produce a stylesheet that has imports (remote url imports) after inlined styles. that means it could produce the problem stylesheet that it itself would refuse to transform:

@import "my-styles.css";
@import "https://example.com/styles.css";

produces

.my-styles {
  /* styles */
}
@import "https://example.com/styles.css";

Yeah, in general, nesting a mix of local and remote imports isn't something that's possible to solve properly (unless you're inlining remote imports as well, which is a whole additional level of complexity).

unless you're inlining remote imports as well, which is a whole additional level of complexity

That is what postcss-import-url does.


In theory this would work :

  • a plugin for remote urls like postcss-import-url doesn't inline but rewrites to a data blob @import url(data:text/css...)
  • this plugin reads data blobs as they are local
  • this plugin is the only plugin that inlines @import

This would be in the spirit of the PostCSS ecosystem by splitting up parts into multiple plugins while using standard syntax as the intermediary.

It would also mean that this plugin doesn't need fundamental changes.
Even separate from this issue I don't know why this plugin shouldn't support blobs.

@argyleink I think this is now mostly resolved.

const importUrl = require('postcss-import-url');
const options = {
  dataUrls: true // this is new ๐ŸŽ‰ 
};

postcss([
  importUrl(options)
]).process(...);
  1. postcss-import-url will inline as base64 encoded data urls.
  2. postcss-import can decode these and inline the "embedded" css.

That should give you the ability to order the @imports purely for your CSS, not for details in PostCSS plugins.