Add ignoreAtRules option to no-invalid-position-at-import #5520
Conversation
There was a problem hiding this comment.
@sylwia-lask Thanks for another pull request!
I've requested some changes (as suggestions, so you can commit them via the GitHub interface).
They align the docs and tests with our conventions, mainly using standard CSS and consistent names. There's a lot of rules in Stylelint, so we do our best to keep things consistent to help our users move through all the documentation.
lib/rules/no-invalid-position-at-import-rule/__tests__/index.js
Outdated
Show resolved
Hide resolved
lib/rules/no-invalid-position-at-import-rule/__tests__/index.js
Outdated
Show resolved
Hide resolved
lib/rules/no-invalid-position-at-import-rule/__tests__/index.js
Outdated
Show resolved
Hide resolved
lib/rules/no-invalid-position-at-import-rule/__tests__/index.js
Outdated
Show resolved
Hide resolved
lib/rules/no-invalid-position-at-import-rule/__tests__/index.js
Outdated
Show resolved
Hide resolved
|
I'm going to create a follow-up issue to update all our |
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
@jeddy3 thanks a million for the code review :) I fixed everything, hope now everything is ok. |
| @@ -27,6 +40,10 @@ function rule(actual) { | |||
| root.walk((node) => { | |||
| const nodeName = node.name && node.name.toLowerCase(); | |||
|
|
|||
| if (node.type === 'atrule' && optionsMatches(options, 'ignoreAtRules', node.name)) { | |||
There was a problem hiding this comment.
Shall we move this into the conditional below to keep everything together, i.e.:
if (
(node.type === 'atrule' &&
nodeName !== 'charset' &&
nodeName !== 'import' &&
!optionsMatches(options, 'ignoreAtRules', node.name) &&
isStandardSyntaxAtRule(node)) ||
(node.type === 'rule' && isStandardSyntaxRule(node))
) {
invalidPosition = true;
return;
}You're right to use node.name rather than nodeName as the ignore* options are case sensitive, and people can use a regex with the i flag if they want case insensitivity.
Incidentally, I wonder whether this rule can be refactored to avoid walking every node. Instead, walking just import at rules, and whether that'll affect performance or not. One for another time, though.
There was a problem hiding this comment.
True, maybe it's worth to refactor that rule. For now I've changed it as you suggest - put every contition together
lib/rules/no-invalid-position-at-import-rule/__tests__/index.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
|
#5492
No, it's self-explanatory.