-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
New Rule: use-layers #18
Comments
I'm not familiar with cascade layers. Are you saying that if there's one |
Thanks for considering this feature. Happy to explain in more detail. The main problem here is that unlayered styles will take precedence over layered styles. When using layers in a codebase, we usually want everything to be in some layer. (The file doesn't matter, it's more about the overall codebase) Here's an example scenario:
One possible complication is that some people might prefer to write unlayered CSS and import it into a layer. I'm not sure if it's possible for eslint to detect that across files. This kind of usage should be ok: @import "unlayered-styles.css" layer(some-layer); As a workaround, users could exclude this eslint rule from being applied on certain files/directories. A related problem that should probably be a separate issue is that it's too easy to misspell a layer name, e.g. |
Got it, thanks. ESLint doesn't work across files, so we only know what's happening in the current file. I can see how this would be valuable, as you probably don't want to mix layered and unlayered styles. And I agree, this shouldn't be enabled by default. So it sounds like we'd just need to flag any rule that isn't inside of a layer? |
I've seen a few different conventions for layers, including intentionally mixing layered and unlayered styles. Perhaps For example, to enforce that:
rules: {
"no-restricted-syntax": [
"error",
{
selector: "Atrule[name=layer]",
message: "The `@layer` at-rule is disallowed. Author unlayered styles instead.",
},
{
selector: "Atrule[name=import]:not(:has(Layer))",
message: "Imports must be put into a layer.",
},
}
} Which would enforce this convention: /* button.css */
.button {
color: red;
} /* styles.css */
@import "bootstrap.css" layer(vendor);
@import "defaults.css" layer(defaults);
@import "button.css" layer(components);
@import "slider.css" layer(components); Or, to enforce the original use case that rules must be within a {
selector:
":matches(StyleSheet > Rule, StyleSheet > Atrule[name!=layer]:has(Rule):not(:has(Atrule[name=layer] Rule)))",
message: "StyleSheet cannot contain unlayered rules",
}, (i.e. flag top-level rules and flag non-layer at-rules which contain rules outside of Which would enforce: @layer foo {
p {
color: red;
}
}
@media (width > 500px) {
@layer bar {
.box {
@layer baz {
padding: 12px;
}
}
}
} People can enforce other Disallowing anonymous layers: {
selector: "Atrule[name=layer]:not(:has(> AtRulePrelude))",
message: "Anonymous layers are disallowed.",
}, @layer {
body {}
} And restricting the name of layers: {
selector: "Layer[name!=/\\b(utilities|components)\\b/]",
message: "Only `utilities` and `components` layers are allowed.",
}, @layer unknown {
body {}
} There are likely more cascading layers conventions people would want to enforce. I'm unfamiliar with ESLint's conventions; when do things become rules, and when are they left to |
@jeddy3 there are downsides of using export default [{
rules: {
"no-restricted-syntax": [
"error",
{
selector: "Atrule[name=layer]",
message: "The `@layer` at-rule is disallowed. Author unlayered styles instead.",
},
{
selector: "Atrule[name=import]:not(:has(Layer))",
message: "Imports must be put into a layer.",
},
}
}
}]; And you'd like to add a custom warning in your config file that inherits from that base config. You'll now need to pull out this definition of Also note that the esquery syntax, while handy, does come with parsing overhead. The longer your selector, the slower the match will be. For any sufficiently long esquery string, it's really a good idea to just create a rule and implement that check in JavaScript. So When there are well-established patterns that would benefit multiple people, we generally look to creating a new rule with configurable options to cover 80-90% of cases. This has the benefit of a rule explicitly saying what it does ( A while back I created a utility to easily create new rules using Let me now if this sounds like a fairly cohesive description for this rule:
Of course, if people want to mix layered and non-layered rules, they wouldn't enable this rule. |
It looks good, except maybe for this parenthetical at the end:
Sometimes it's fine to have an anonymous layer inside a named layer, so |
Good call out, I agree. 👍 I think I'd also probably call this |
Thank you for taking the time to explain the downsides of
If someone uses cascade layers via For example: /* defaults.css */
a {
color: pink;
}
button {
color: red;
&:hover {
color: orange;
}
} /* primary-button.css */
.primary-button {
color: blue;
} /* secondary-button.css */
.seconday-button {
color: green;
} /* styles.css */
@import "defaults.css" layer(defaults);
@import "primary-button.css" layer(components);
@import "secondary-button.css" layer(components); I think people take this approach so that:
Can the I'm not sure the Would moving the For example,
And something like
The |
Personally, I do use both. My setup involves |
That's fascinating. I did wonder if people used both approaches to nest their layers. Is there a way to wrangle the options so that the rule supports both use cases?:
I wonder if there are more use cases, too. It feels like there aren't established patterns around cascade layers yet. |
Keep in mind that ESLint functions on only one file at a time. If you're using the pattern where all of your files don't contain layers except for your top-level file that imports everything into layers, then you'd only enable import css from "@eslint/css";
export default [
{
plugins: {
css
},
files: ["styles/*.css"],
...css.configs.recommended
},
{
plugins: {
css
},
files: ["styles/index.css"],
rules: {
"css/use-layers": "error"
}
}
]; |
I forgot about overrides, sorry 😅 Would it make sense to tweak the description to:
So that @mayank99's use case is enabled without options: import css from "@eslint/css";
export default [
{
plugins: {
css
},
files: ["styles/*.css"],
...css.configs.recommended
},
{
rules: {
"css/use-layers": "error"
}
}
]; i.e. use cascade layers everywhere (both to wrap rules and when importing). So is the import css from "@eslint/css";
export default [
{
plugins: {
css
},
files: ["styles/*.css"],
...css.configs.recommended
},
{
plugins: {
css
},
files: ["styles/index.css"],
rules: {
"css/use-layers": "error"
}
}
]; And the same for an import css from "@eslint/css";
export default [
{
plugins: {
css
},
files: ["styles/*.css"],
...css.configs.recommended
},
{
plugins: {
css
},
files: ["styles/components/**/.css"],
rules: {
"css/use-layers": "error"
}
}
]; |
Rule details
Flag any CSS rules that are outside a
@layer
block.What type of rule is this?
Warns about a potential problem
Example code
This will be flagged:
This is fine:
Note: Nested layers can used as
@layer foo.bar {…}
and anonymous layers can be used as@layer {…}
. Both of these are fine and should not be flagged.Some CSS, like
@import
, is fine to keep outside a@layer
, because the contents of the file would still be using@layer
.Participation
Additional comments
When working with cascade layers, unlayered styles are usually a mistake. It is easy to forget to wrap the styles in
@layer
, which is exactly the kind of thing a linter should help with.This rule probably should be opt-in, because cascade layers aren't being widely used yet.
The text was updated successfully, but these errors were encountered: