Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add ignoreAfterCombinators: [] to selector-max-universal #6275

Merged
merged 10 commits into from Aug 18, 2022
Merged

Add ignoreAfterCombinators: [] to selector-max-universal #6275

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c3a355a
tests
fpetrakov Aug 16, 2022
45bce81
add option ignoreAfterCombinators
fpetrakov Aug 17, 2022
8c00178
add docs and improve tests
fpetrakov Aug 17, 2022
c157b12
add support for user defined combinators
fpetrakov Aug 17, 2022
f8982a1
doc fix
fpetrakov Aug 17, 2022
0610d1f
one more doc fix
fpetrakov Aug 17, 2022
f752f30
made use of prev method on childNode + added check descendant combina…
fpetrakov Aug 17, 2022
67e4c18
tests and docs fixes
fpetrakov Aug 18, 2022
65ebc48
fixes
fpetrakov Aug 18, 2022
50c610d
revert doc change
fpetrakov Aug 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions lib/rules/selector-max-universal/README.md
View file
Open in desktop
Expand Up @@ -74,3 +74,24 @@ The following patterns are _not_ considered problems:
/* `*` is inside `:not()`, so it is evaluated separately */
* > * .foo:not(*) {}
```

## Optional secondary options

### `ignoreAfterCombinators: ["array", "of", "combinators"]`

Ignore universal selectors that come after one of the specified combinators.

Given:

```json
[">", "+"]
```

For example, with `2`.

The following pattern is _not_ considered a problem:

<!-- prettier-ignore -->
```css
* * > * {}
```
97 changes: 97 additions & 0 deletions lib/rules/selector-max-universal/__tests__/index.js
View file
Open in desktop
Expand Up @@ -186,6 +186,18 @@ testRule({
code: '* { @media print { * {} } }',
description: 'media query: nested',
},
{
code: '* + * {}',
description: 'adjacent sibling combinator compound selector',
},
{
code: '* > * {}',
description: 'child combinator compound selector',
},
{
code: '* ~ * {}',
description: 'general sibling combinator compound selector',
},
],

reject: [
Expand All @@ -198,6 +210,35 @@ testRule({
endLine: 1,
endColumn: 6,
},
{
code: '* + * + * {}',
description:
'adjacent sibling combinator compound selector: greater than max universal selectors',
message: messages.expected('* + * + *', 2),
line: 1,
column: 1,
endLine: 1,
endColumn: 10,
},
{
code: '* > * > * {}',
description: 'child combinator compound selector: greater than max universal selectors',
message: messages.expected('* > * > *', 2),
line: 1,
column: 1,
endLine: 1,
endColumn: 10,
},
{
code: '* ~ * ~ * {}',
description:
'general sibling combinator compound selector: greater than max universal selectors',
message: messages.expected('* ~ * ~ *', 2),
line: 1,
column: 1,
endLine: 1,
endColumn: 10,
},
{
code: '*, \n* * * {}',
description: 'multiple selectors: greater than max universal selectors',
Expand Down Expand Up @@ -228,6 +269,62 @@ testRule({
],
});

testRule({
ruleName,
config: [1, { ignoreAfterCombinators: ['>'] }],

accept: [
{
code: '*.foo {}',
},
{
code: '*.foo > * {}',
},
],

reject: [
{
code: '*.foo * {}',
message: messages.expected('*.foo *', 1),
line: 1,
column: 1,
endLine: 1,
endColumn: 8,
},
],
});

testRule({
ruleName,
config: [0, { ignoreAfterCombinators: ['~', '+', '>'] }],

accept: [
{
code: '.foo {}',
},
{
code: '.foo ~ * {}',
},
{
code: '.foo + * {}',
},
{
code: '.foo > * {}',
},
],

reject: [
{
code: '* {}',
message: messages.expected('*', 0),
line: 1,
column: 1,
endLine: 1,
endColumn: 2,
},
],
});

testRule({
ruleName,
config: [0],
Expand Down
39 changes: 33 additions & 6 deletions lib/rules/selector-max-universal/index.js
View file
Open in desktop
Expand Up @@ -8,6 +8,8 @@ const resolvedNestedSelector = require('postcss-resolve-nested-selector');
const ruleMessages = require('../../utils/ruleMessages');
const selectorParser = require('postcss-selector-parser');
const validateOptions = require('../../utils/validateOptions');
const optionsMatches = require('../../utils/optionsMatches');
const { isString } = require('../../utils/validateTypes');

const ruleName = 'selector-max-universal';

Expand All @@ -23,12 +25,23 @@ const meta = {
};

/** @type {import('stylelint').Rule} */
const rule = (primary) => {
const rule = (primary, secondaryOptions) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: primary,
possible: isNonNegativeInteger,
});
const validOptions = validateOptions(
result,
ruleName,
{
actual: primary,
possible: isNonNegativeInteger,
},
{
actual: secondaryOptions,
possible: {
ignoreAfterCombinators: [isString],
},
optional: true,
},
);

if (!validOptions) {
return;
Expand All @@ -46,7 +59,21 @@ const rule = (primary) => {
checkSelector(childNode, ruleNode);
}

if (childNode.type === 'universal') total += 1;
let prevChildNode = childNode.prev();
let prevChildNodeValue = prevChildNode && prevChildNode.value;

// checks if previous child node is descendant combinator
if (prevChildNode && prevChildNode.type === 'combinator' && prevChildNodeValue === ' ') {
prevChildNode = prevChildNode.prev();

prevChildNodeValue = prevChildNode && prevChildNode.value;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What purpose does this bit of code have?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What purpose does this bit of code have?

As you know, there are 4 types of combinators, according to MDN. This code's purpose is to add support for descendant combinator. This way user can add smth like this to the configuration: ["h1"].

For example, we have code like this: a > * { color: red }. With combinators ">", "+", "~" it's very easy to check, in this case we did it this way:
let prevChildNode = childNode.prev();
let prevChildNodeValue = prevChildNode && prevChildNode.value;
prevChildNodeValue will equal to ">", "+" or "~", and then we just pass the value to optionMatches function:
optionsMatches(secondaryOptions, 'ignoreAfterCombinators', prevChildNodeValue)

If we have code h1 * { color: red } and we want to ignore * after h1, we will stumble upon descendant combinator, which is represented by a single space (" ") character. Inn this case, the code from above will not work as prevChildNoveValue will equal to " ". We have to go back even farther and check previous node of the prevChildNode to get the value h1.

Copy link
Member

@jeddy3 jeddy3 Aug 18, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. I think we can remove this code.

This code's purpose is to add support for the descendant combinator.

I believe the rule supports the descendant combinator without this code, via the configuration of [" "]. We could probably do more to accommodate other whitespace descendant combinators (like tabs and newlines), but let's only do that if someone requests it. By supporting ">", "+", "~" and " " (space) we satisfy the most common use cases and the original one in #5792.

If we have code h1 * { color: red } and we want to ignore * after h1

We can introduce a separate ignoreAfterSelector: [] secondary option to support this as h1 is a type selector, not a combinator (and ["h1"] shouldn't be a valid configuration for the ignoreAfterCombinators: [] option). Let's only add this option if someone requests it, though.


if (childNode.type === 'universal') {
if (!optionsMatches(secondaryOptions, 'ignoreAfterCombinators', prevChildNodeValue)) {
total += 1;
}
}

return total;
}, 0);
Expand Down