Fix declaration-property-value-keyword-no-deprecated false positives for arguments of unknown functions#8579
Fix declaration-property-value-keyword-no-deprecated false positives for arguments of unknown functions#8579Mouvedia wants to merge 2 commits into
declaration-property-value-keyword-no-deprecated false positives for arguments of unknown functions#8579Conversation
🦋 Changeset detectedLatest commit: 298a282 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 |
|
This PR is packaged and the instant preview is available (298a282). View the demo website. Install it locally: npm i -D https://pkg.pr.new/stylelint@298a282 |
7390f1c to
679b463
Compare
679b463 to
6626a7a
Compare
There was a problem hiding this comment.
| @@ -17,11 +17,14 @@ export const camelCaseFunctions = new Set([ | |||
| export const colorFunctions = new Set([ | |||
There was a problem hiding this comment.
6626a7a to
05c4644
Compare
…s for arguments of unknown functions
05c4644 to
f92f068
Compare
| 'contrast-color', | ||
| 'device-cmyk', | ||
| 'hsl', | ||
| 'hsla', |
There was a problem hiding this comment.
e.g. hsla(from infotext h s l)
ref https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_colors/Relative_colors
romainmenke
left a comment
There was a problem hiding this comment.
Thank you for working on this @Mouvedia
| return; | ||
| } | ||
|
|
||
| if (node.type === 'function' && !colorFunctions.has(node.value.toLowerCase())) { |
There was a problem hiding this comment.
Is it correct that we actually want to check if background is intended as a <color> or as some unrelated argument to a function?
a {
/* background as color */
color: rgb(from background r g b);
}
a {
/* background as color */
color: var(--foo, background);
}
a {
/* background as color */
color: rgb(from var(--foo, background) r g b);
}
a {
/* background as color */
color: rgb(from if(style(--foo: bar): background; else: white) r g b);
}
a {
/* background as a random keyword */
color: if(style(--foo: background): red; else: white);
}
a {
/* background as a random keyword */
color: --custom-function(background);
}This would also be easier to keep track of with the state feature of @csstools/css-parser-algorithms.
When entering a function like var(), if(), rgb(), ccolor-mix(), contrast-color(), ... is encountered the checks for values like background would run.
When entering unrelated functions like style(), --custom-function(), ... the checks would skip.
There was a problem hiding this comment.
Out of the properties/values supported only the color functions are relevant.
Also the user would expect the rule to pick the deprecated keyword within those functions.
e.g.
calc-size doesn't support intrinsic AFAIK so it doesn't need an exception because it would be invalid in the first place. Now the actual wording in the specification is not really precise:
The production matches any intrinsic size keywords allowed in the context.
so I might be wrong.
Granted, it would be much simpler to ignore all functions.
There was a problem hiding this comment.
off topic
l132
Ignored keywords may erroneously not be ignored in some cases.
i.e. it should be done inside parsedValue.walk((node) => { … }); too
It won't be fixed in this PR though.
|
@romainmenke concerning #8579 (comment) |
Closes #8575
N/A