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 media-feature-name-value-no-unknown
#6906
Add media-feature-name-value-no-unknown
#6906
Conversation
…e-value-no-unknown--witty-south-china-tiger-8b25eed791
🦋 Changeset detectedLatest commit: 85a24c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…ty-south-china-tiger-8b25eed791
…ty-south-china-tiger-8b25eed791
da58881
to
ad00a17
Compare
There are probably some cases taken from this list that could be covered by additional tests . |
ruleName, | ||
config: [true], | ||
|
||
accept: [ |
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.
screen-spanning
is an unknown media feature and those are ignored by this rule (tested with (foo: ...)
).
media-feature-name-no-unknown
already exists to cover those issues.
Good call @Mouvedia and thank you for reviewing all this 🙇 I've added all the examples from that link. That link had an example of |
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
const units = new Set(require('../../reference/units').units); // a copy that is safe to mutate | ||
|
||
// `x` as a resolution unit is very often a typo for `px`. | ||
// By removing it from the set of known units, we can catch those typos. | ||
// Intentional `x` units are supported by manually checking these in specific functions or properties. | ||
units.delete('x'); |
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.
see : #6906 (comment)
Ignoring x
is very specific to this one rule.
If we want to have a re-usable reference for resolutionUnits
and if resolutionUnits
are included in units
, then we must move the removal of x
to this rule.
I think this is fine because it makes the exception local to the rule that requires it.
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.
see also: #3026 (comment)
lib/rules/media-feature-name-value-no-unknown/__tests__/index.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
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 the great work. LGTM 👏🏼
Thank you everyone for the extensive review! 🙇 |
This new rule might be in our recommended config when the rule is mature. 😄 |
Closes #6560
No, it's self-explanatory.
Tested this with a large monorepo containing ±50 websites and got no false positives and it tracked down a real error :