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 declaration-property-value-no-unknown
false negatives for typed custom properties
#7078
Conversation
… custom properties
🦋 Changeset detectedLatest commit: 7185f35 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 |
lib/rules/declaration-property-value-no-unknown/__tests__/index.mjs
Outdated
Show resolved
Hide resolved
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
…lse-negatives-for-typed-custom-properties--philosophical-borneo-elephant-8c41a1b24a
@romainmenke Is this intentionally a draft, or can we review it? |
Intentionally a draft :) But early feedback is always welcome if there are obvious issues/improvements |
// https://github.com/mdn/data/pull/674 | ||
// `initial-value` has a incorrect syntax definition. | ||
// In reality everything is valid. | ||
if ( | ||
/^initial-value$/i.test(prop) && | ||
decl.parent && | ||
isAtRule(decl.parent) && | ||
/^property$/i.test(decl.parent.name) | ||
) { | ||
return; | ||
} |
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.
I didn't cache the regexp's here because this entire block should be removed.
I don't mind optimizing the code anyway.
But I am really hoping that csstree will receive some attention in the near future.
…lse-negatives-for-typed-custom-properties--philosophical-borneo-elephant-8c41a1b24a
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.
@romainmenke Thanks for the Pull Request! I think this is great. 👍🏼
I've commented on a few refactoring suggestions, but they're minor things. 😃
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
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.
Thank you. LGTM! 👍🏼
Closes #7061
I chose not to validate the contents of the
syntax
property in a@property
rule.It is always valid if it also works in a browser.
It might be invalid in browsers and still be processed by CSSTree as a valid syntax definition.
I think this is fine as it future proofs this rule.
We will not have to change this rule each time the restrictions here are relaxed and more of the full value definition syntax becomes allowed.