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

New Rule: no-id-selector #19

Open
1 task done
aryan02420 opened this issue Nov 27, 2024 · 11 comments · May be fixed by #24
Open
1 task done

New Rule: no-id-selector #19

aryan02420 opened this issue Nov 27, 2024 · 11 comments · May be fixed by #24
Labels

Comments

@aryan02420
Copy link

aryan02420 commented Nov 27, 2024

Rule details

Flag any CSS selector that uses the ID selector

What type of rule is this?

Warns about a potential problem

Example code

#one { /* lint error */
  background-color: yellow;
}

h1#heading { /* lint error */
  color: rebeccapurple;
}

The #id selector has very high specificity and doesn't always play well with other rules. An alternative is to use a .class selector instead.

When using a component-based UI framework, id selectors are often not very useful. You can hardcode an id into a component, but this makes the component non-reusable. The framework-generated unique ids cannot be referenced in CSS. Therefore, using id selectors in these cases is likely a mistake.

Participation

  • I am willing to submit a pull request to implement this rule.

Additional comments

From MDN:

The ID selector has high specificity. This means styles applied based on matching an ID selector will overrule styles applied based on other selector, including class and type selectors. Because an ID can only occur once on a page and because of the high specificity of ID selectors, it is preferable to add a class to an element instead of an ID. If using the ID is the only way to target the element — perhaps because you do not have access to the markup and cannot edit it — consider using the ID within an attribute selector, such as p[id="header"].

@jeddy3
Copy link

jeddy3 commented Nov 27, 2024

A CSS equivalent to JavaScript's no-restricted-syntax rule would cover this use case and more.

We couldn't take this approach in Stylelint because the general parser we use is limited to a few node types. We ended up with many separate rules, including selector-max-id, that disallow or limit things. We turn on only a handful of these in our standard config because we found restricting CSS syntax is rarely universal. There is also the stylelint-no-restricted-syntax plugin that can target these few node types.

The CSSTree parser is far more granular and id selectors are a node type (IdSelector):

{
    "rules": {
        "no-restricted-syntax": [
            "error",
            {
                "selector": "IdSelector",
                "message": "Id selectors are not allowed."
            },
        ]
    }
}

(Mixing AST selectors and CSS selector types could be confusing for people, though.)

The ID selector has high specificity... If using the ID is the only way to target the element — perhaps because you do not have access to the markup and cannot edit it — consider using the ID within an attribute selector, such as p[id="header"].

A rule to limit specificity would complement the no-restricted-syntax one. In Stylelint, we have selector-max-specificity, and people use it when they want to limit the specificity of selectors.

@jeddy3 jeddy3 mentioned this issue Nov 27, 2024
1 task
@nzakas
Copy link
Member

nzakas commented Nov 27, 2024

A rule to limit specificity would complement the no-restricted-syntax one. In Stylelint, we have selector-max-specificity, and people use it when they want to limit the specificity of selectors.

Ah interesting, that may be a better way to go than just limited blocking of ID selectors.

@nzakas
Copy link
Member

nzakas commented Nov 27, 2024

Looking into this right now. Would it make sense to ever set a minimum specificity? For instance, would a rule like this have any value?

specificity: ["error". {
    max: [0, 3, 5],
    min: [0, 1, 0]
}

@nzakas nzakas linked a pull request Nov 27, 2024 that will close this issue
1 task
@nzakas
Copy link
Member

nzakas commented Nov 27, 2024

Rough draft: #24

@NotWoods
Copy link

As a +1 to a restricted syntax rule, I'd also want to ban some tag selectors. These selectors tend to be slow with common elements:

.foo > div {} /* must evaluate every div */

or they get abused by developers instead of using classes

dialog > div > span:nth-child(1) > img {}

@aryan02420
Copy link
Author

or they get abused by developers instead of using classes

dialog > div > span:nth-child(1) > img {}

I agree. Classes are always better than tag selectors and less prone to breakage

@fasttime fasttime added this to Triage Nov 28, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Nov 28, 2024
@jeddy3
Copy link

jeddy3 commented Nov 29, 2024

Not everyone would use a no-id-selector rule as there's a diverse set of CSS methodologies, and type selectors are sometimes a good fit, especially when the CSS is scoped.

For example, with the @scope at-rule:

@scope (.component) to (.content) {
  a {
    color: red;
  }
}

Or within the Shadow Dom using (Lit) Web Components:

import {LitElement, html, css} from 'lit';
import {customElement} from 'lit/decorators.js';

@customElement('my-element')
export class MyElement extends LitElement {
  static styles = css`
    p {
      color: green;
    }
  `;
  protected render() {
    return html`<p>I am green!</p>`;
  }
}

The nice thing about a no-restricted-syntax rule is that people can configure it for their preferred approach. For example, if they use something like CSS modules for scoping, they'd probably want to disallow:

  • id selectors
  • type selectors
  • top-level combinators
{
    "rules": {
        "no-restricted-syntax": [
            "error",
            {
                "selector": "IdSelector",
                "message": "Id selectors are not allowed."
            },
            {
                "selector": "TypeSelector",
                "message": "Type selectors are not allowed."
            },
            {
                "selector": "Rule > SelectorList > Selector > Combinator",
                "message": "Top-level combinators are not allowed."
            },
        ]
    }
}

By only disallowing top-level combinators, you'll still be able to use them in powerful pseudo-classes like :has(). It would allow things like this:

.component {
  &:hover {}
}

.child {
  &:has(+ .alt) {}
}

While disallowing things like:

#id {
 & > div {
    & > div {}
  }
}

A no-restricted-syntax rule would cover many of Stylelint's disallow and limit rules, including selector-max-id and selector-combinator-disallowed-list.

@jeddy3
Copy link

jeddy3 commented Dec 1, 2024

It turns out that the core no-restricted-syntax rule can already be used with CSS!

You can disallow id and type selectors with:

import css from "@eslint/css";

export default [
  {
    files: ["**/*.css"],
    language: "css/css",
    ...css.configs.recommended,
  },
  {
    rules: {
      "no-restricted-syntax": [
        "error",
        {
          selector: "IdSelector",
          message: "Id selectors are not allowed.",
        },
        {
          selector: "TypeSelector",
          message: "Type selectors are not allowed.",
        },
      ],
    },
  },
];

@nzakas
Copy link
Member

nzakas commented Dec 3, 2024

Thinking out loud: maybe what we're really after is a rule that limits the complexity of selectors? If that was the original intent of selector-max-specificity, then maybe a rule like selector-complexity could serve that purpose?

We could have options for:

  • maximum specificity
  • disallow ID selectors
  • disallow type selectors
  • disallow :is
  • disallow :where

Would that make sense?

@jeddy3
Copy link

jeddy3 commented Dec 4, 2024

Selector complexity in CSS is an odd one because it's:

  • a sliding scale of what you get for your money
  • multifaceted with a lot of features in play

It seems that the way people use selectors is a broad spectrum. On one side, there are people who:

  • only use class selectors
  • avoid combinators
  • avoid functional pseudo-classes
  • avoid selector lists

People at the other end lean into all the selector features. For example, using :has() with other functional pseudo-classes and combinators to move styling logic out of JavaScript into CSS:

/* selecting cards that don't have an `img` within */
.card:not(:has(img)) {}

/* selecting boxes that have 3 or more items */
.box:has(.item:nth-last-child(n + 3)) {}

/* selecting boxes that preceded a circle */
.box:has(~ .circle) {}

Or, using :is() to remove duplication in selector lists:

/* this */
header :is(h1, h2) a {}

/* instead of this */
header h1 a,
header h2 a {}

Or, using type selectors within @scope:

@scope (.component) to (.content) {
  a {
    color: red;
  }
}

And then there's everyone in between those ends of the spectrum.

Realising that there isn't a one-size-fits-all when it comes to selector use, we added a bunch of rules to Stylelint that people can use to enforce their conventions based on their ideas of complexity and consistency. They can use these rules to limit the:

And a rule to disallow qualifying, and a general one to disallow patterns in selectors.

We also added allowed versions of many of the list rules, e.g. allowing a subset of combinators, but, in hindsight, this is a less common use case which you can probably avoid.

Some may see complexity as the number of compound selectors or combinators, whereas for someone else, it may be the number of different types of selectors used (id, class, universal and so on). It'd be great to accommodate both with this rule. Rather than single out id and type selectors and the :is() and :where() pseudo-classes, can it provide options to limit the number and types of the different selectors and combinators?

@nzakas
Copy link
Member

nzakas commented Dec 4, 2024

Rather than single out id and type selectors and the :is() and :where() pseudo-classes, can it provide options to limit the number and types of the different selectors and combinators?

Yes, that was actually my intent. I just included some examples off the top of my head in my previous comment.

It seems like a rule that allows everything to start and you have to configure it to disallow something might make sense? So maybe you'd have a "max" option for each part of a selector (i.e., maxIds, maxClasses, etc.)?

@nzakas nzakas moved this from Needs Triage to Evaluating in Triage Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Evaluating
Development

Successfully merging a pull request may close this issue.

4 participants