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
Add ignoreSelectors option to property-no-unknown #4275
Add ignoreSelectors option to property-no-unknown #4275
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.
@dprgarner Thanks for the PR!
I've made some documentation requests.
@@ -130,3 +130,25 @@ a { | |||
-moz-overflow-scrolling: center; | |||
} | |||
``` | |||
|
|||
### `ignoreSelectors: ["/regex/", /regex/, "string"]` |
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.
Let's move this under the ignoreProperties
docs so these similar options are grouped.
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.
👍
|
||
### `ignoreSelectors: ["/regex/", /regex/, "string"]` | ||
|
||
Some CSS extensions allow arbitrary child properties of certain selectors and pseudo-classes, which shouldn't be checked against a whitelist of known properties. One such example is [ICSS](https://github.com/css-modules/icss), which defines `:export` and `:import` pseudo-classes allowing any valid CSS property name to be nested in their declaration blocks. |
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.
We use standard CSS syntax in our docs. Code related to methodologies or language extensions can confuse users unfamiliar with them. Also, methodologies like ICSS will come and go, but CSS will stick around. Using plain CSS in our examples has made our docs resilient over the past 5 years.
I think we can strip this paragraph out and just use something like :root
in the example as ignoreProperties
already shows an example of regex usage. For example:
ignoreSelectors: ["/regex/", /regex/, "string"]
Given:
[":root"]
The following patterns are not considered violations:
:root {
my-property: blue;
}
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.
Makes sense.
// Is the property a child of an ignored selector? | ||
const { selector } = decl.parent; | ||
|
||
if (selector && optionsMatches(options, "ignoreSelectors", selector)) { |
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.
Thanks for PR, I think it will be better to move this check right after ignoreProperties
check. This is up to you.
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.
Makes sense. Benchmarks look the same.
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.
Thanks for making the changes. LGTM.
|
Closes #4270
I think it's okay as-is. It's implementing what was discussed on the issue, with docs and tests (and a small typo fix).