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
Fix no-descending-specificity
false positives for pseudo-classes
#6195
Fix no-descending-specificity
false positives for pseudo-classes
#6195
Conversation
*/ | ||
function checkSelector(selectorNode, ruleNode, comparisonContext) { | ||
const selector = selectorNode.toString(); | ||
const referenceSelectorNode = lastCompoundSelectorWithoutPseudoClasses(selectorNode); | ||
const referenceSelector = lastCompoundSelectorWithoutPseudoClasses(selectorNode); |
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.
[note] lastCompoundSelectorWithoutPseudoClasses()
returns a string, not a node. So I think referenceSelector
is a more readable name.
); | ||
}); | ||
|
||
if (nodesWithoutPseudoClasses.length === 0) return undefined; |
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.
[note] If nodesWithoutPseudoClasses
is empty, the old version of this function returns an empty string (''
). This makes it more difficult to handle irregular values. So the current code returns undefined
instead.
I created another example, But I'm unsure whether it has anything to do with this problem. |
@richex-cn Thank you for the advice! I've added a test case via 423095a, and this fix works in that case. 👍🏼 |
see https://github.com/stylelint/stylelint/blob/main/docs/user-guide/rules/about.md#user-defined-ignore |
@Mouvedia Thanks for providing the guidance. Indeed, I think it may be better that built-in rules don't support non-standard syntaxes like CSS Modules for simplicity and maintainability. |
I don't have an opinion. I am just saying let's be consistent. |
Just getting back around to this. It's a can of worms and I don't think I've fully grokked it yet. I think we want to address functional pseudo-classes more generally rather than just the non-standard The rule currently considers the following comparable when it should not: a :where(a, b) {}
:where(c, d) {} I haven't dug into the implementation of the rule deeply, but I think we should be including the Then, as (We could add a different option which lets a user specify a list of functional pseudo-class to treat like |
Let's add an |
@jeddy3 Thanks for the suggestion. I mistakenly thought the CSS Modules specific code was required; indeed, it's unnecessary. See the commit 68bcaa which makes the test suite pass. If the |
no-descending-specificity
false positives for :global()
pseudo-classno-descending-specificity
false positives for pseudo-classes
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.
LGTM, thank you!
If the ignoreSelectors option you suggested may be needed in the future, but in this pull request, it's not needed for now.
👍
Closes #4010
This pull request aims to fix false positives for the
no-descending-specificity
rule about the CSS Modules:global()
pseudo-class.I'm unfamiliar with CSS Modules, so I'm unsure whether this added code specific to
:global()
is suitable. Perhaps, someone may know a more smart solution.In addition, I don't know if we should address the
:local()
pseudo-class. Please let me know if someone knows anything about it.See also #4010 (comment) and the reproduction demo.