Fix false positives for namespaced variables in property-no-unknown #4803
Conversation
| // Sass variables within a namespace (e.g. namespace.$var: x) | ||
| if (prop.includes('.$') && prop[0] !== '.' && prop[prop.length - 1] !== '$') { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
I'm not sure what this function should return for invalid declarations such as .$var and namespace.$. For now this branch excludes those invalid props.
There was a problem hiding this comment.
I think that the logic about SCSS variables added via this PR can be unified with the following code:
stylelint/lib/utils/isStandardSyntaxProperty.js
Lines 12 to 15 in 6cfea29
stylelint/lib/utils/isStandardSyntaxDeclaration.js
Lines 32 to 35 in 6cfea29
For example, what about adding a new utility function to handle SCSS variables?
There was a problem hiding this comment.
@ryym Thanks for the pull request. I've made a request.
I think that the logic about SCSS variables added via this PR
Yes, we should remove that duplication.
@ryym Are you able to create a isScssVariable(prop) util that returns true for either:
prop.startsWith('$'))prop.includes('.$')
Then use that util in both:
isStandardSyntaxDeclaration.jsisStandardSyntaxProperty.js.
| @@ -57,5 +57,10 @@ module.exports = function (decl) { | |||
| return false; | |||
| } | |||
|
|
|||
| // Sass variables within a namespace (e.g. namespace.$var: x) | |||
| if (prop.includes('.$') && prop[0] !== '.' && prop[prop.length - 1] !== '$') { | |||
There was a problem hiding this comment.
Let's just go with if (prop.includes('.$')) as the util is more interested in excluding non-standard properties than the validity of name-spaced dollar variables.
|
@ybiquitous @jeddy3 Thanks for your reviewing! |
|
Updated changelog:
|
# By Mike Allanson (6) and others # Via GitHub * master: Bump got from 11.2.0 to 11.3.0 (#4825) Export an object from the CSS-in-JS syntax (#4824) Add type `Formatter` for formatter functions (#4823) Update CHANGELOG.md Fix false positives for namespaced variables in property-no-unknown (#4803) Update CHANGELOG.md Fix TypeError for inline comments and autofix for sugarss in max-empty-lines (#4821) Create new 'lintPostcssResult' module (#4819) 13.6.0 Update deps Prepare 13.6.0 Update CHANGELOG.md
Closes #4544
I didn't add test cases of
isStandardSyntaxDeclarationfor Sass indented syntax because thepostcss-sassparser does not understand@usesyntax yet (AleshaOleg/postcss-sass#145).