-
-
Notifications
You must be signed in to change notification settings - Fork 934
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
Fix false positives on non-standard syntax in declaration-block-no-* #3381
Conversation
code: | ||
"margin-left: 10px; margin-right: 10px; margin-top: 20px; margin-bottom: 30px;", | ||
message: messages.expected("margin") | ||
}, |
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 add test with html and style tag
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.
Done.
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.
@gucong3000 Thanks for starting this.
I've added two comments about the tests. The comments apply to all the files e.g. I don't believe the following is valid syntax:
padding-top: 1px; padding-bottom: 1px; a { padding-left: 1px; padding-right: 1px; }
@@ -18,6 +18,10 @@ testRule(rule, { | |||
code: "a { color: pink; { &:hover { color: orange; } } }", | |||
description: "spec nested" | |||
}, | |||
{ | |||
code: "color: pink; @media { color: orange; }", |
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.
As per the developer guide:
Ensure you use standard CSS syntax by default
I don't believe a declaration outside of rule-set is valid. What are we trying to test here?
If it's the content of a style=""
attribute then how does this differ from the syntax: html
tests below which are more explicit?
@@ -48,6 +52,10 @@ testRule(rule, { | |||
], | |||
|
|||
reject: [ | |||
{ | |||
code: "color: pink; color: orange", |
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.
If this is for style=""
attributes? If so, how does it differ from the syntax: html
tests below?
cd4cd67
to
c4c6c69
Compare
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.
@gucong3000 Thanks for moving the tests to their own testRule
. It's much clearer what's going on now and aligns with the checklist:
Ensure you use standard CSS syntax by default, and only swap parsers when testing a specific piece of non-standard syntax.
@evilebottnawi How does this PR now look to you?
@evilebottnawi If you've time to review this today, then I'll be able to do a release before I go away for the weekend. |
@jeddy3 LGTM |
|
194: Update dependency stylelint to v9.3.0 r=rehandalal a=renovate[bot] This Pull Request updates dependency [stylelint](https://github.com/stylelint/stylelint) from `v9.2.1` to `v9.3.0` <details> <summary>Release Notes</summary> ### [`v9.3.0`](https://github.com/stylelint/stylelint/blob/master/CHANGELOG.md#​930) [Compare Source](stylelint/stylelint@9.2.1...9.3.0) - Added: support for `<style>` tags and `style=""` attributes in XML and XSLT files ([#​3386](`https://github.com/stylelint/stylelint/pull/3386`)). - Added: `globbyOptions` option ([#​3339](`https://github.com/stylelint/stylelint/pull/3339`)). - Added: `keyframes-name-pattern` rule ([#​3321](`https://github.com/stylelint/stylelint/pull/3321`)). - Added: `media-feature-name-value-whitelist` rule ([#​3320](`https://github.com/stylelint/stylelint/pull/3320`)). - Added: `selector-pseudo-element-colon-notation` autofix ([#​3345](`https://github.com/stylelint/stylelint/pull/3345`)). - Fixed: `.vue` files throwing errors for `<style lang="stylus">` and `<style lang="postcss">` ([#​3331](`https://github.com/stylelint/stylelint/pull/3331`)). - Fixed: `declaration-block-no-*` false positives for non-standard syntax ([#​3381](`https://github.com/stylelint/stylelint/pull/3381`)). - Fixed: `function-whitespace-after` false positives for "/" ([#​3132](`https://github.com/stylelint/stylelint/pull/3132`)). - Fixed: `length-zero-no-unit` incorrect autofix for at-includes ([#​3347](`https://github.com/stylelint/stylelint/pull/3347`)). - Fixed: `max-nesting-depth` false positives for nested properties ([#​3349](`https://github.com/stylelint/stylelint/pull/3349`)). - Fixed: `no-empty-source` false positives on vue external sources `<style src="*">` tag ([#​3331](`https://github.com/stylelint/stylelint/pull/3331`)). - Fixed: `max-line-length` false positives for non-CSS blocks ([#​3367](`https://github.com/stylelint/stylelint/pull/3367`)). - Fixed: `no-eol-whitespace` false positives for non-CSS blocks ([#​3367](`https://github.com/stylelint/stylelint/pull/3367`)). - Fixed: `no-extra-semicolons` false positives for non-CSS blocks ([#​3367](`https://github.com/stylelint/stylelint/pull/3367`)). - Fixed: `no-missing-end-of-source-newline` false positives for non-CSS blocks ([#​3367](`https://github.com/stylelint/stylelint/pull/3367`)). --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com). Co-authored-by: Renovate Bot <bot@renovateapp.com>
This Pull Request updates dependency [stylelint](https://github.com/stylelint/stylelint) from `v9.2.1` to `v9.3.0` <details> <summary>Release Notes</summary> ### [`v9.3.0`](https://github.com/stylelint/stylelint/blob/master/CHANGELOG.md#​930) [Compare Source](stylelint/stylelint@9.2.1...9.3.0) - Added: support for `<style>` tags and `style=""` attributes in XML and XSLT files ([#​3386](`https://github.com/stylelint/stylelint/pull/3386`)). - Added: `globbyOptions` option ([#​3339](`https://github.com/stylelint/stylelint/pull/3339`)). - Added: `keyframes-name-pattern` rule ([#​3321](`https://github.com/stylelint/stylelint/pull/3321`)). - Added: `media-feature-name-value-whitelist` rule ([#​3320](`https://github.com/stylelint/stylelint/pull/3320`)). - Added: `selector-pseudo-element-colon-notation` autofix ([#​3345](`https://github.com/stylelint/stylelint/pull/3345`)). - Fixed: `.vue` files throwing errors for `<style lang="stylus">` and `<style lang="postcss">` ([#​3331](`https://github.com/stylelint/stylelint/pull/3331`)). - Fixed: `declaration-block-no-*` false positives for non-standard syntax ([#​3381](`https://github.com/stylelint/stylelint/pull/3381`)). - Fixed: `function-whitespace-after` false positives for "/" ([#​3132](`https://github.com/stylelint/stylelint/pull/3132`)). - Fixed: `length-zero-no-unit` incorrect autofix for at-includes ([#​3347](`https://github.com/stylelint/stylelint/pull/3347`)). - Fixed: `max-nesting-depth` false positives for nested properties ([#​3349](`https://github.com/stylelint/stylelint/pull/3349`)). - Fixed: `no-empty-source` false positives on vue external sources `<style src="*">` tag ([#​3331](`https://github.com/stylelint/stylelint/pull/3331`)). - Fixed: `max-line-length` false positives for non-CSS blocks ([#​3367](`https://github.com/stylelint/stylelint/pull/3367`)). - Fixed: `no-eol-whitespace` false positives for non-CSS blocks ([#​3367](`https://github.com/stylelint/stylelint/pull/3367`)). - Fixed: `no-extra-semicolons` false positives for non-CSS blocks ([#​3367](`https://github.com/stylelint/stylelint/pull/3367`)). - Fixed: `no-missing-end-of-source-newline` false positives for non-CSS blocks ([#​3367](`https://github.com/stylelint/stylelint/pull/3367`)). --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).
#3380
emotion-js/emotion#686 (comment)
No, it's self explanatory.