-
Notifications
You must be signed in to change notification settings - Fork 115
layers support #483
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
layers support #483
Conversation
@romainmenke See how we do it for media imports here: Lines 79 to 83 in e2d2890
|
@RyanZim Thank you for the hint! I think this is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question; otherwise looks good. BTW, sorry so long in getting around to reviewing this.
) { | ||
return result.warn( | ||
"@import must precede all other statements (besides @charset)", | ||
"@import must precede all other statements (besides @charset or empty @layer)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not saying this is wrong, but can you show where the spec allows for empty @layer
before @import
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://drafts.csswg.org/css-cascade-5/#at-import
Any @import rules must precede all other valid at-rules and style rules in a style sheet (ignoring @charset and empty @layer definitions) and must not have any other valid at-rules or style rules between it and previous @import rules, or else the @import rule is invalid.
https://drafts.csswg.org/css-cascade-5/#layer-block
Note: @layer block at-rules cannot be interleaved with @import rules.
In more detail : https://drafts.csswg.org/css-cascade-5/#layer-empty
The @layer rule can also be used to define new layers without assigning any style rules, by providing only the layer name:
@layer #;
Such empty @layer rules are allowed before @import and @namespace rules (after the @charset rule, if any) as well as everywhere @layer block at-rules are allowed.
But now that I have some distance since writing this I think that empty @layer
is not a useful way to describe this to stylesheet authors.
I had to look it up, both to check that I didn't make a mistake and what the exact meaning was.
Is there a better, more widely used name for at-rules without a block?
@layer foo; /* a good name for this? */
@import 'baz.css' layer(baz);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if there's a good name for it; I think following the spec's wording is a safe bet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works for me!
We can also see if this causes confusion to authors and adjust if needed.
It might be a non issue as people learn the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RyanZim Is this resolved or do you want me to change something?
Not in any rush to have this shipped, but wasn't sure :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All set, just need to merge and release this when I get a chance.
Just thought about it, perhaps there should be some mention of this in the README? |
Yes! |
In the meantime; released in v14.1.0! 🎉 |
Nice! Just used it to double check the example for the readme! Thank you for all the reviews and guidance 🎉 |
With
@layer
support soon in all browsers stylesheet authors will also start using these in@import
.spec : https://drafts.csswg.org/css-cascade-5/#at-import
new syntax :
Because I am not familiar with the codebase I first wanted to checkin to make sure I am on the right track.
Status :
layer
The intention is to have this transform :
I could not easily find the best place to intervene and create a new
atRule
for the layers. Is this something you can help out with @ai?