Fix false negatives for css-in-js object notation in color-hex-case #5101
Conversation
As discussed in stylelint#4826, `style-search` has false positives when used with `css-in-js` object notation. This change updates the `color-hex-case` rule to use `postcss-value-parser` instead, which has support for `css-in-js` as a syntax. The unit tests now also include a suite for testing both `upper` and `lower` options for `color-hex-case` with the `css-in-js` syntax.
|
This is my first contribution, so let me know if there's something I missed with respect to formatting or code style. |
|
Thanks for taking the time to look at all of this @jeddy3! I've gone through and made the changes you requested. For many of them (like the invalid five hex colors, e.g. Should we transform the original tests to also use more valid CSS? Or just leave them as they were before? |
There was a problem hiding this comment.
Fantastic, thanks for making the changes! It's almost there. Just one last tweak with the indents.
Should we transform the original tests to also use more valid CSS? Or just leave them as they were before?
Let's leave them as they are. We default to using valid CSS, but deviating in this case makes sense as it's testing something particular. Whereas in the testRule for "css-in-js" were testing that objects are read correctly, rather than the edge cases of the rule itself, and so valid CSS is appropriate here.
|
Thanks! I was gonna start in on some of the other Should I wait until this one gets the second approval and merged? Also, would you prefer one PR per rule? |
`style-search` returns false positives when used with `css-in-js` object notation. Continuing on the change made in stylelint#5101, this updates `color-hex-length` to switch to using `postcss-value-parser`. Unit tests have also been added to check the `css-in-js` syntax for this rule.
Fantastic!
No need to wait. Yes, one pull request per rule, please. |
lib/rules/color-hex-case/index.js
Outdated
|
|
||
| return stringStart + replaceString + stringEnd; | ||
| return decl; |
There was a problem hiding this comment.
Return is not used anywhere.
There was a problem hiding this comment.
@Dru89 It looks like this is the only change we need to make for a second approval.
There was a problem hiding this comment.
@Dru89 It looks like this is the only change we need to make for a second approval.
jeddy3
changed the title
|
Sorry for the delay in getting this updated. Last week was a bit busier than I expected, but I've addressed that last piece of feedback. |
…#5106) * Use postcss-value-parser for color-hex-length `style-search` returns false positives when used with `css-in-js` object notation. Continuing on the change made in #5101, this updates `color-hex-length` to switch to using `postcss-value-parser`. Unit tests have also been added to check the `css-in-js` syntax for this rule. * Remove unnecessary return
As discussed in #4826,
style-searchhas false positives when used withcss-in-jsobject notation. This change updates thecolor-hex-caserule to usepostcss-value-parserinstead, which has support forcss-in-jsas a syntax.The unit tests now also include a suite for testing both
upperandloweroptions forcolor-hex-casewith thecss-in-jssyntax.