-
-
Notifications
You must be signed in to change notification settings - Fork 44
feat: add no-unknown-animations rule #535
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| # no-unknown-animations | ||
|
|
||
| Disallow unknown animation names. | ||
|
|
||
| ## Background | ||
|
|
||
| CSS animations are created by assigning a [`@keyframes`](https://developer.mozilla.org/en-US/docs/Web/CSS/@keyframes) rule's name to the [`animation-name`](https://developer.mozilla.org/en-US-US/docs/Web/CSS/animation-name) property or the [`animation`](https://developer.mozilla.org/en-US/docs/Web/CSS/animation) shorthand property, as in this example: | ||
|
|
||
| ```css | ||
| .card { | ||
| animation: fade-in 300ms ease; | ||
| } | ||
|
|
||
| @keyframes fade-in { | ||
| from { | ||
| opacity: 0; | ||
| } | ||
|
|
||
| to { | ||
| opacity: 1; | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| If an animation name doesn't match any `@keyframes` rule, for example because of a typo or because the `@keyframes` rule was renamed or removed, the animation silently fails to run without any error. | ||
|
|
||
| ## Rule Details | ||
|
|
||
| This rule warns when an animation name used in `animation` or `animation-name` doesn't match any `@keyframes` rule defined in the same source. | ||
|
|
||
| Animation names are case-sensitive, and quoted and unquoted names refer to the same animation, so `animation-name: "fade-in"` matches `@keyframes fade-in`. | ||
|
|
||
| The rule only checks statically determinable animation names. Dynamic animation names, such as those using `var()`, are ignored. | ||
|
|
||
| Examples of **incorrect** code for this rule: | ||
|
|
||
| ```css | ||
| /* eslint css/no-unknown-animations: "error" */ | ||
|
|
||
| .card { | ||
| animation: fade-in 300ms ease; | ||
| } | ||
|
|
||
| .button { | ||
| animation-name: slide-up; | ||
| } | ||
|
|
||
| @keyframes fade-out { | ||
| from { | ||
| opacity: 1; | ||
| } | ||
|
|
||
| to { | ||
| opacity: 0; | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Examples of **correct** code for this rule: | ||
|
|
||
| ```css | ||
| /* eslint css/no-unknown-animations: "error" */ | ||
|
|
||
| .card { | ||
| animation: fade-in 300ms ease; | ||
| } | ||
|
|
||
| .button { | ||
| animation-name: slide-up; | ||
| } | ||
|
|
||
| @keyframes fade-in { | ||
| from { | ||
| opacity: 0; | ||
| } | ||
|
|
||
| to { | ||
| opacity: 1; | ||
| } | ||
| } | ||
|
|
||
| @keyframes slide-up { | ||
| from { | ||
| transform: translateY(8px); | ||
| } | ||
|
|
||
| to { | ||
| transform: translateY(0); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## When Not to Use It | ||
|
|
||
| Animations can reference `@keyframes` rules defined in another stylesheet, but this rule only checks `@keyframes` rules defined in the same source. If your `@keyframes` rules are defined separately from where the animations are used, you should not use this rule. | ||
|
|
||
| ## Prior Art | ||
|
|
||
| - [`no-unknown-animations`](https://stylelint.io/user-guide/rules/no-unknown-animations/) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,141 @@ | ||||||
| /** | ||||||
| * @fileoverview Rule to disallow unknown animation names. | ||||||
| * @author Gaic4o | ||||||
| */ | ||||||
|
|
||||||
| //----------------------------------------------------------------------------- | ||||||
| // Type Definitions | ||||||
| //----------------------------------------------------------------------------- | ||||||
|
|
||||||
| /** | ||||||
| * @import { CSSRuleDefinition } from "../types.js" | ||||||
| * @import { CssLocationRange } from "@eslint/css-tree" | ||||||
| * @typedef {"unknownAnimation"} NoUnknownAnimationsMessageIds | ||||||
| * @typedef {CSSRuleDefinition<{ RuleOptions: [], MessageIds: NoUnknownAnimationsMessageIds }>} NoUnknownAnimationsRuleDefinition | ||||||
| */ | ||||||
|
|
||||||
| //----------------------------------------------------------------------------- | ||||||
| // Helpers | ||||||
| //----------------------------------------------------------------------------- | ||||||
|
|
||||||
| const animationPropertyPattern = /^animation(?:-name)?$/iu; | ||||||
|
|
||||||
| /** | ||||||
| * Extracts an animation name from a node. Quoted and unquoted animation | ||||||
| * names refer to the same animation, so `"fade-in"` and `fade-in` both | ||||||
| * yield `fade-in`. | ||||||
| * @param {Object} node The node to extract the animation name from. | ||||||
| * @returns {string|null} The animation name, or `null` if the node isn't a name. | ||||||
| */ | ||||||
| function getAnimationName(node) { | ||||||
| if (node.type === "Identifier") { | ||||||
| return node.name; | ||||||
| } | ||||||
|
|
||||||
| if (node.type === "String") { | ||||||
| return node.value; | ||||||
| } | ||||||
|
|
||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| //----------------------------------------------------------------------------- | ||||||
| // Rule Definition | ||||||
| //----------------------------------------------------------------------------- | ||||||
|
|
||||||
| export default /** @satisfies {NoUnknownAnimationsRuleDefinition} */ ({ | ||||||
| meta: { | ||||||
| type: "problem", | ||||||
|
|
||||||
| docs: { | ||||||
| description: "Disallow unknown animation names", | ||||||
| recommended: false, | ||||||
| url: "https://github.com/eslint/css/blob/main/docs/rules/no-unknown-animations.md", | ||||||
| }, | ||||||
|
|
||||||
| messages: { | ||||||
| unknownAnimation: "Unknown animation name '{{name}}' found.", | ||||||
| }, | ||||||
| }, | ||||||
|
|
||||||
| create(context) { | ||||||
| const lexer = context.sourceCode.lexer; | ||||||
|
|
||||||
| /** @type {Set<string>} */ | ||||||
| const definedAnimations = new Set(); | ||||||
|
|
||||||
| /** @type {Array<{ name: string, loc: CssLocationRange }>} */ | ||||||
| const usedAnimations = []; | ||||||
|
|
||||||
| return { | ||||||
| "Atrule[name=/^(-(o|moz|webkit)-)?keyframes$/i] > AtrulePrelude"( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The "ms" vendor-prefix is also missing here. Please also add a test case for this. |
||||||
| node, | ||||||
| ) { | ||||||
| const child = node.children[0]; | ||||||
| const name = child ? getAnimationName(child) : null; | ||||||
|
|
||||||
| if (name !== null) { | ||||||
| definedAnimations.add(name); | ||||||
| } | ||||||
| }, | ||||||
|
|
||||||
| "Rule > Block Declaration"(node) { | ||||||
| if ( | ||||||
| !animationPropertyPattern.test(node.property) || | ||||||
| node.value.type !== "Value" | ||||||
| ) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| const matchResult = lexer.matchProperty( | ||||||
| node.property, | ||||||
| node.value, | ||||||
| ); | ||||||
|
|
||||||
| /* | ||||||
| * 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 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the rule should support checking local resolvable
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 For example, 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ended up implementing One thing I’d like your opinion on is that this implementation re-parses the value using |
||||||
| * values are outside the scope of this rule. | ||||||
| */ | ||||||
| if (matchResult.error) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| for (const child of node.value.children) { | ||||||
| if (!matchResult.isType(child, "keyframes-name")) { | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| const name = getAnimationName(child); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the I kept the check on the |
||||||
|
|
||||||
| if (name !== null) { | ||||||
| usedAnimations.push({ | ||||||
| name, | ||||||
| loc: child.loc, | ||||||
| }); | ||||||
| } | ||||||
| } | ||||||
| }, | ||||||
|
|
||||||
| /* | ||||||
| * Usages are reported only after the entire stylesheet has been | ||||||
| * visited so that `@keyframes` rules defined after their usage | ||||||
| * are still found. | ||||||
| */ | ||||||
| "StyleSheet:exit"() { | ||||||
| for (const { name, loc } of usedAnimations) { | ||||||
| if (definedAnimations.has(name)) { | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| context.report({ | ||||||
| loc, | ||||||
| messageId: "unknownAnimation", | ||||||
| data: { name }, | ||||||
| }); | ||||||
| } | ||||||
| }, | ||||||
| }; | ||||||
| }, | ||||||
| }); | ||||||
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.
This should also check for vendored prefixes (e.g.
-webkit-animation) as the@keyframescheck also does.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’ve addressed the issue you pointed out. Thank you!