feat: add no-unknown-animations rule - #535
Conversation
| // Helpers | ||
| //----------------------------------------------------------------------------- | ||
|
|
||
| const animationPropertyPattern = /^animation(?:-name)?$/iu; |
There was a problem hiding this comment.
This should also check for vendored prefixes (e.g. -webkit-animation) as the @keyframes check also does.
There was a problem hiding this comment.
I’ve addressed the issue you pointed out. Thank you!
| /* | ||
| * If the value can't be matched against the property grammar, | ||
| * its animation name can't be determined reliably. This | ||
| * includes dynamic values such as var(). Invalid property |
There was a problem hiding this comment.
I think the rule should support checking local resolvable var declarations.
There will be a helper for this but this rule could already check the default value of a var, e.g. "slide-in" in animation: var(--animation-name, "slide-in").
There was a problem hiding this comment.
Thanks for pointing this out. After looking into it, I think it makes more sense for this rule to check values that can be determined statically, rather than trying to fully resolve every var() usage.
For example, var(--animation-name) would still be ignored when its actual value cannot be determined, while cases such as var(--animation-name, "slide-in") could be checked by extracting the statically known animation name from the fallback value.
For resolving local custom property values themselves, I think it would be better not to implement that separately in this PR, and instead make use of the helper you mentioned once it is available. So for this PR, I’m planning to support checking statically resolvable fallback values first.
There was a problem hiding this comment.
I ended up implementing var() handling a little more broadly than I initially described. Even when a value contains var(), the rule now checks fallback values as well as any statically known animation names around it. Actual custom property value resolution is still something I plan to handle later using the helper you mentioned.
One thing I’d like your opinion on is that this implementation re-parses the value using parse() from @eslint/css-tree. Since this does not use the custom parser when customSyntax is configured, I’d like to know whether you think this approach is okay.
| continue; | ||
| } | ||
|
|
||
| const name = getAnimationName(child); |
There was a problem hiding this comment.
The name should not be null as the lexer already checks that it is a string or an identifier. Otherwise a test case for this is missing.
There was a problem hiding this comment.
I removed the null check on the usage side. As you pointed out, the lexer only matches an identifier or a string as <keyframes-name>, so it can't be null in this case.
I kept the check on the @keyframes prelude side, though. This part isn't validated by the lexer, so @keyframes 50% can be parsed as Percentage and @keyframes 1s as Dimension. I also added tests for both cases.
2 participants
AI acknowledgment
What is the purpose of this pull request?
This PR adds the
no-unknown-animationsrule to report animation names that don't match any@keyframesrule defined in the same source.What changes did you make? (Give an overview)
no-unknown-animationsrule foranimationandanimation-namedeclarations.Related Issues
fixes #529
Disclosure: I'm a participant of open source contribution program OSSCA