-
-
Notifications
You must be signed in to change notification settings - Fork 26
feat: add selector-complexity
rule
#252
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
base: main
Are you sure you want to change the base?
Conversation
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.
Excited to see this rule taking shape. I noticed that currently the rule only reports on a single node in the selector:

This is different from Stylelint, which reports on the whole selector (#foo #bar #baz
) when a it finds a problem:
I believe that reporting on the whole selector would be less confusing because it's not obvious which part of a selector should be changed to comply with the rule. Thoughts?
*/ | ||
function exceedLimitError(context, selectorsList, maxValue, selectorType) { | ||
context.report({ | ||
loc: selectorsList[maxValue].loc, |
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.
For a descendant combinator the loc
property is null
, which is not allowed by context.report
.
For example, this test case is failing with an error:
{
code: ".foo .bar {}",
options: [{ maxCombinators: 0 }],
errors: 1,
}
1) selector-complexity
invalid
.foo .bar {}:
Error: Node must be provided when reporting error if location is not provided
Occurred while linting <input>:1
Rule: "rule-to-test/selector-complexity"
at ok (node_modules/eslint/lib/shared/assert.js:17:9)
at assertValidNodeInfo (node_modules/eslint/lib/linter/file-report.js:173:3)
at FileReport.addRuleMessage (node_modules/eslint/lib/linter/file-report.js:538:3)
at FileContext.report (node_modules/eslint/lib/linter/linter.js:1111:28)
at exceedLimitError (file:///.../css/src/rules/selector-complexity.js:81:10)
at Selector (file:///.../css/src/rules/selector-complexity.js:352:6)
at ruleErrorHandler (node_modules/eslint/lib/linter/linter.js:1173:33)
at /.../css/node_modules/eslint/lib/linter/source-code-visitor.js:76:46
at Array.forEach (<anonymous>)
at SourceCodeVisitor.callSync (node_modules/eslint/lib/linter/source-code-visitor.js:76:30)
at /.../css/node_modules/eslint/lib/linter/source-code-traverser.js:291:18
at Array.forEach (<anonymous>)
at SourceCodeTraverser.traverseSync (node_modules/eslint/lib/linter/source-code-traverser.js:290:10)
at runRules (node_modules/eslint/lib/linter/linter.js:1214:12)
at #flatVerifyWithoutProcessors (node_modules/eslint/lib/linter/linter.js:2101:4)
at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (node_modules/eslint/lib/linter/linter.js:2189:43)
at Linter._verifyWithFlatConfigArray (node_modules/eslint/lib/linter/linter.js:2292:15)
at Linter.verify (node_modules/eslint/lib/linter/linter.js:1677:10)
at runRuleForItem (node_modules/eslint/lib/rule-tester/rule-tester.js:884:23)
at testInvalidTemplate (node_modules/eslint/lib/rule-tester/rule-tester.js:1075:19)
at Context.<anonymous> (node_modules/eslint/lib/rule-tester/rule-tester.js:1564:10)
at process.processImmediate (node:internal/timers:485:21)
I think in this and other cases it would be better to report on the whole selector (.foo .bar
).
Prerequisites checklist
What is the purpose of this pull request?
To add a new rule to limit CSS selectors.
What changes did you make? (Give an overview)
Added a new rule called
selector-complexity
with following options:maxIds
maxClasses
maxTypes
maxAttributes
maxPseudoClasses
maxUniversals
maxCompounds
maxCombinators
disallowCombinators
disallowPseudoClasses
disallowPseudoElements
disallowAttributes
disallowAttributeMatchers
Related Issues
Fixes #19
Is there anything you'd like reviewers to focus on?