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

Add support for evaluation contexts pseudo-classes in selector-max-specificity #2857

Closed
Hypnosphi opened this Issue Sep 6, 2017 · 8 comments

Comments

3 participants
@Hypnosphi
Contributor

Hypnosphi commented Sep 6, 2017

Describe the issue. Is it a bug or a feature request (new rule, new option, etc.)?

False positive when using CSS modules

Which rule, if any, is this issue related to?

selector-max-specificity

What CSS is needed to reproduce this issue?

.lets.goto :local(.bar) {
  color: pink;
}

What stylelint configuration is needed to reproduce this issue?

{
  "rules": {
    "selector-max-specificity": "0,3,0"
  }
}

Which version of stylelint are you using?

8.0.0

How are you running stylelint: CLI, PostCSS plugin, Node API?

CLI with stylelint --ignore-path .eslintignore '**/*.css'

Does your issue relate to non-standard syntax (e.g. SCSS, nesting, etc.)?

Yes, it's related to CSS modules

What did you expect to happen?

No warnings to be flagged.

What actually happened (e.g. what warnings or errors you are getting)?

The following warnings were flagged:

✖  Expected ".lets.goto :local(.bar)" to have a specificity no more than "0,3,0"   selector-max-specificity

Figure out what needs to be done, propose it

Ignore :local and :global, as it's done with :not and :matches

@ntwb ntwb added the type: bug label Sep 6, 2017

@ntwb

This comment has been minimized.

Member

ntwb commented Sep 6, 2017

Thanks for the detailed issue and using our issue template @Hypnosphi 😄

I've marked this up as a bug and I'll submit a PR for this momentarily 🎉

@ntwb ntwb self-assigned this Sep 6, 2017

@jeddy3 jeddy3 changed the title from CSS Modules pseudoclasses `:global` and `:local` shouldn't count when measuring specificity to Add ignorePseudoClasses: [] to selector-max-specificity Sep 9, 2017

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Sep 9, 2017

@Hypnosphi This can be resolved by adding a new ignorePsuedoClasses: [] secondary option to the rule. Please consider contributing this option if you have time. There's a section in the Developer Guide on how to do it. There's also a similar option in selector-pseudo-class-no-unknown: code, code, README, and tests.

I've updated the labels and title of the issue accordingly.

I also have a pending PR that documents this design decision.

@Hypnosphi

This comment has been minimized.

Contributor

Hypnosphi commented Sep 9, 2017

Why any selector containing :not or :matches is completely ignored? This leads to false negatives when you have a selector like .a.b.c.d.e.f:not(.g).

BTW, looks like specificity knows how to handle :not: https://github.com/keeganstreet/specificity/blob/master/specificity.js#L105-L111
At the same time, :matches isn't supported there (they only support CSS Selectors Level 3)

@jeddy3 jeddy3 unassigned ntwb Sep 9, 2017

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Sep 9, 2017

BTW, looks like specificity knows how to handle :not:

Yes, you're right. In, #1504 it looks like we underestimated specificity's support for :not(). We should have only ignored :not() when it contains anything more complex than a simple selector. I'll create a new issue for that so this issue can stay focused on the new option.

At the same time, :matches isn't supported there (they only support CSS Selectors Level

Yes, until specificity supports it we are best ignoring it outright.

@Hypnosphi

This comment has been minimized.

Contributor

Hypnosphi commented Sep 9, 2017

In fact, :local and :global selectors should work exactly like :not, i.e. they shouldn't count when calculating specificity, but their content should. So looks like we have to do some parsing here

I noticed that some other rules use postcss-selector-parser via parseSelector util, maybe we could just pass all the simple selectors to specificity and calculate the sum, or even calculate the specificity ourselves

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Sep 10, 2017

In fact, :local and :global selectors should work exactly like :not, i.e. they shouldn't count when calculating specificity, but their content should.

True.

maybe we could just pass all the simple selectors to specificity and calculate the sum

I think this is worth pursuing (and in the rule itself, rather than as a plugin, as it should bring Level 4 not() support too).

Is this something that you're willing to explore further and contribute? It might even be worth contributing Level 4 support upstream in the specificity package.

@jeddy3 jeddy3 changed the title from Add ignorePseudoClasses: [] to selector-max-specificity to Fix false negatives for evaluation contexts pseudo-classes in selector-max-specificity Sep 10, 2017

@Hypnosphi

This comment has been minimized.

Contributor

Hypnosphi commented Sep 10, 2017

I think I can give it a try (in this package)

@jeddy3 jeddy3 changed the title from Fix false negatives for evaluation contexts pseudo-classes in selector-max-specificity to Add support for evaluation contexts pseudo-classes in selector-max-specificity Sep 10, 2017

@jeddy3 jeddy3 removed the type: bug label Sep 10, 2017

@ntwb

This comment has been minimized.

Member

ntwb commented Sep 12, 2017

Fixed in 2279fd5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment