Avoid comments in at-rule-property-requirelist #4744
Conversation
| @@ -38,9 +38,16 @@ function rule(primary) { | |||
| } | |||
|
|
|||
| primary[atRuleName].forEach((property) => { | |||
| if (!property) { | |||
There was a problem hiding this comment.
Is this needed? property in this context is the property from the configuration object, e.g. margin in :
{
"config": { "page": ["margin"] }
}If I take it out locally, the tests still pass.
There was a problem hiding this comment.
Good catch; I must have gotten my line numbers mixed up.
| const propertyName = property.toLowerCase(); | ||
|
|
||
| const hasProperty = nodes.find((node) => node.prop.toLowerCase() === propertyName); | ||
| const hasProperty = nodes.find( |
There was a problem hiding this comment.
Let's go with this minor tweak:
const hasProperty = nodes.find(
({type, prop}) => type === "decl" && prop.toLowerCase() === propertyName
);The original issue was caused by our assumption that an at-rule would only have declarations for children. By using type === "decl" we are being explicit about wanting to check declarations, whereas checking for the existence of the prop property only implies that. I think the former may better communicate the intent.
I'll create a follow-up pull request that tweaks the working on rules docs to suggest using this approach when node.forEach is a better fit than node.walk*.
|
Changelog:
|
Fixes #4638
It's self-explanatory.
If the
undefinedvalues represent a bug upstream, I'd be glad for a hint as to which project that might be.