fix import with layer

[romainmenke]

fixes : #495

@RyanZim it still feels a bit like the complexity (which I added) is exploding and the overal shape of the code base isn't optimal. But I also do not dislike this fix :)

Important spec bits : https://www.w3.org/TR/css-cascade-5/#at-import

the optional layer keyword or layer() function assigns the contents of the style sheet into its own anonymous cascade layer or into the named cascade layer.

The layer is added to the layer order even if the import fails to load the stylesheet, but is subject to any import conditions (just as if declared by an @layer rule wrapped in the appropriate conditional group rules).the optional layer keyword or layer() function assigns the contents of the style sheet into its own anonymous cascade layer or into the named cascade layer.

The layer is added to the layer order even if the import fails to load the stylesheet, but is subject to any import conditions (just as if declared by an @layer rule wrapped in the appropriate conditional group rules).

Which means that @import url('foo.css') layer(A) (min-width: 320px) becomes :

@media (min-width: 320px) {
  @layer A {
    /* imported styles */
  }
}

[romainmenke]

@RyanZim do you have time one of the following days to review this? 🙇

[RyanZim]

@romainmenke My apologies for my slowness here; I've been quite busy. Then, I was in a fairly serious accident last week, so that's put things even more on behind. I have not forgotten; I will try to review in the coming days.

[romainmenke]

@RyanZim Sorry to hear that :/
Take all the time you need, this is just code and not important.

I hope you are ok 💐

[RyanZim]

I'll be alright, I'm mostly back to normal already. Again, apologies for the slow action here.

There's one part of the code I don't understand (left a comment below). Also, it seems there's some unnecessary opening and closing of statements in the output (comments below), is there a reason for this?

[romainmenke]

@RyanZim Thank you for the review and happy to hear things are getting better on your end 🎉

[RyanZim]

I'm hesitant about the naming of anonymous layers, and what unexpected problems that may create. I'm not super familiar with @layer myself, so I don't know all the edge cases.

[romainmenke]

I'm hesitant about the naming of anonymous layers, and what unexpected problems that may create. I'm not super familiar with @layer myself, so I don't know all the edge cases.

I actually am also unsure about this.
Thank you for calling this out.

We could make anonymous layered imports unsupported for now.
That way we avoid unforeseen edge cases while still shipping an improved version.

We can iterate on this later to add support for anonymous layers.
This would only affect @import "foo.css" layer; not @layer {}.

The current version might be incorrect when an author has multiple stylesheets, all with anonymous layered imports. There is no way for this plugin to be aware of that and the counter approach doesn't produce truly unique layer names.

I did however want to avoid using true random values as these make builds non-reproducible.

---

From a technical standpoint an anonymous layer is equivalent to a named layer that only appears once.

So a sufficiently uniquely named layer is the same as an anonymous layer.

Making this sufficiently unique is the tricky part.

[RyanZim]

OK, compromise proposal: we don't support anonymous layers by default. To enable support, you must pass an option which is a function, that is passed the index, and must return the layer name. That way, if you want our current index solution, you could pass:

(index) => `imported-anon-layer-${index}`

If you have multiple stylesheets, you could use different prefixes. Maybe we should also pass the root filename we're processing, for those who want to incorporate that into the name. Alternately, if someone doesn't care about deterministic builds, they could simply provide a random string generator (with or without a prefix).

I realize this is a bit complicated, but it also provides maximum flexibility; thoughts? Also, sane naming suggestions for such an option?

[romainmenke]

I like this!
I will try to implement this in the next few days :)

[romainmenke]

@RyanZim I think this is ready for review (when you have time).
For the moment I have settled on nameLayer for the plugin option.

Other candidates :

• nameLayer
• deAnonymizeLayer
• generateLayerName

---

I wasn't sure how to pass the root file name considering that it might not be set.
At the moment I am using the import file name as a second argument.

[RyanZim]

I wasn't sure how to pass the root file name considering that it might not be set.
At the moment I am using the import file name as a second argument.

I think the root file name is way more useful than the import file name. It should be an optional argument; i.e. if the root file name isn't set, nameLayer just gets one argument, index.

What is the actual output like if you don't pass layerName, but use anonymous layers? I'm wondering if we should be erroring instead of warning there: https://github.com/postcss/postcss/blob/main/docs/guidelines/plugin.md#41-use-nodeerror-on-css-relevant-errors

[romainmenke]

@RyanZim Thank you for the feedback.
I've made the necessary changes.

Hopefully all good now :)

[RyanZim]

Merged; this is technically semver-major, so I'm going to try to bundle this with a Node version requirement bump to release.

[romainmenke]

Thank you for this @RyanZim 🙇

[RyanZim]

Released in v15