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
Incorrect tests for specificity and css modules in selector-max-specificity
#7495
Comments
@romainmenke Thanks for the report. In addition, thanks for trying |
It has been decided at one point that we wouldn't care about CSS modules selectors. |
Regressions or CSS modules compat are not really my primary concern here :) CSS modules are supported in But the tests contain incorrect CSS for CSS modules. :global(.page, .bar)
:global(.page, :local(.bar)) Because these are bogus we can safely assume that no user will be writing this and that changes won't cause regressions. At the same time these test are the only coverage we have for There is no coverage with standard functional pseudo classes and I don't know what the calculated specificity should be for these :
.foo :is(#bar, input) {} // [1, 1, 0] without ignore Is that :
The same is true for
This is different from non-functional pseudo classes. .foo :focus {} // [0, 2, 0] without ignore Ignoring |
I agree that we should correct the current weird behavior of |
Probably, I guess the following behavior would be correct: ⬇️ Given: {"ignoreSelectors": [":global"]} Then: :global {} /* ignored */
:global(.foo) {} /* not ignored */ Given: {"ignoreSelectors": ["/:global\\(.+\\)/"]} Then: :global {} /* not ignored */
:global(.foo) {} /* ignored */ |
Would it be ok to remove the bogus tests and to replace them with tests for standard functional pseudo classes? Then at least there is coverage for the current implementation in a way that makes it clear what the expectations are. |
If I had to guess it was to cover a real use case; this seems reasonable to me.
If that's a documentation update please check #4105 as well.
If true, 👍 |
Our end goal is to modernise and make our behaviour around specificity and nesting spec-compliant, which will benefit the majority of our users. As such, I think we can remove barriers that'll hamper us in achieving this for our next major release, including removing options that may no longer be relevant. I suggest we remove the
If not, a person can open a new issue with their use case, and we'll decide on the most modern way to cater to it.
We've typically avoided broad ignore options like We can reach for similar options to cater to specific use cases. |
I agree with replacing Perhaps, I assume that CSS Modules selectors like E.g. {
"selector-max-specificity": ["0,2,0", {"ignoreFunctionalPseudoClasses": [":global", ":local"]}]
} .foo .bar.baz {} /* '0,3,0' => reported */
:global(.foo) .bar.baz {} /* '0,2,0' => not reported */ So, I suggest the following steps:
|
Sounds reasonable. In the meantime
shouldn't hinder that plan. The documentation update could be skipped since we are planning to replace the option. |
see :
stylelint/lib/rules/selector-max-specificity/__tests__/index.mjs
Line 451 in 382961f
These tests are problematic for multiple reasons:
:global
and:local
are non-standard and it isn't defined what their specificity is.:global
and:local
don't have any specificity of their own in@csstools/selector-specificity
, so ignoring them to reduce their specificity seems weird?:global
and:local
do not support selector lists in implementationslightningcss
errorspostcss-modules
mangles the output (:global(.page, .bar)
->.page .bar
)They also dominate the implementation of
selector-max-specificity
.As far as I can tell the
ignoreSelectors
was also created with css modules too much in mind.It acts like a hole punch. The AST node that matches doesn't contribute to specificity, but its child nodes do. Which works really well for
:global
and:local
because they happen to be removed during compilation.But I am not sure if this makes sense for any case other than css modules.
There isn't any test coverage for other pseudo selectors with child nodes, so I can't tell what the exact intention was here.
Proposal :
:global
and:local
are used as supported by implementationsignoreSelectors
works when it matches AST nodes with child nodesThe text was updated successfully, but these errors were encountered: