From 57d184f2839810d01bc0e97a5e0470125729ea8c Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Thu, 22 May 2025 00:29:19 +0530 Subject: [PATCH 1/7] feat: add new rule `no-duplicate-keyframe-selectors` --- .gitignore | 1 + README.md | 23 +- docs/rules/no-duplicate-keyframe-selectors.md | 91 +++++++ eslint.config.js | 8 +- src/index.js | 3 + src/rules/no-duplicate-keyframe-selectors.js | 73 +++++ .../no-duplicate-keyframe-selectors.test.js | 249 ++++++++++++++++++ 7 files changed, 436 insertions(+), 12 deletions(-) create mode 100644 docs/rules/no-duplicate-keyframe-selectors.md create mode 100644 src/rules/no-duplicate-keyframe-selectors.js create mode 100644 tests/rules/no-duplicate-keyframe-selectors.test.js diff --git a/.gitignore b/.gitignore index fdf8c75..c5615f5 100644 --- a/.gitignore +++ b/.gitignore @@ -135,3 +135,4 @@ dist package-lock.json yarn.lock bun.lockb +test.css diff --git a/README.md b/README.md index af29a52..335f6c3 100644 --- a/README.md +++ b/README.md @@ -65,17 +65,18 @@ export default defineConfig([ -| **Rule Name** | **Description** | **Recommended** | -| :----------------------------------------------------------------------- | :------------------------------------- | :-------------: | -| [`no-duplicate-imports`](./docs/rules/no-duplicate-imports.md) | Disallow duplicate @import rules | yes | -| [`no-empty-blocks`](./docs/rules/no-empty-blocks.md) | Disallow empty blocks | yes | -| [`no-important`](./docs/rules/no-important.md) | Disallow !important flags | yes | -| [`no-invalid-at-rules`](./docs/rules/no-invalid-at-rules.md) | Disallow invalid at-rules | yes | -| [`no-invalid-properties`](./docs/rules/no-invalid-properties.md) | Disallow invalid properties | yes | -| [`prefer-logical-properties`](./docs/rules/prefer-logical-properties.md) | Enforce the use of logical properties | no | -| [`relative-font-units`](./docs/rules/relative-font-units.md) | Enforce the use of relative font units | no | -| [`use-baseline`](./docs/rules/use-baseline.md) | Enforce the use of baseline features | yes | -| [`use-layers`](./docs/rules/use-layers.md) | Require use of layers | no | +| **Rule Name** | **Description** | **Recommended** | +| :----------------------------------------------------------------------------------- | :-------------------------------------------------- | :-------------: | +| [`no-duplicate-imports`](./docs/rules/no-duplicate-imports.md) | Disallow duplicate @import rules | yes | +| [`no-duplicate-keyframe-selectors`](./docs/rules/no-duplicate-keyframe-selectors.md) | Disallow duplicate selectors within keyframe blocks | yes | +| [`no-empty-blocks`](./docs/rules/no-empty-blocks.md) | Disallow empty blocks | yes | +| [`no-important`](./docs/rules/no-important.md) | Disallow !important flags | yes | +| [`no-invalid-at-rules`](./docs/rules/no-invalid-at-rules.md) | Disallow invalid at-rules | yes | +| [`no-invalid-properties`](./docs/rules/no-invalid-properties.md) | Disallow invalid properties | yes | +| [`prefer-logical-properties`](./docs/rules/prefer-logical-properties.md) | Enforce the use of logical properties | no | +| [`relative-font-units`](./docs/rules/relative-font-units.md) | Enforce the use of relative font units | no | +| [`use-baseline`](./docs/rules/use-baseline.md) | Enforce the use of baseline features | yes | +| [`use-layers`](./docs/rules/use-layers.md) | Require use of layers | no | diff --git a/docs/rules/no-duplicate-keyframe-selectors.md b/docs/rules/no-duplicate-keyframe-selectors.md new file mode 100644 index 0000000..2a9aaad --- /dev/null +++ b/docs/rules/no-duplicate-keyframe-selectors.md @@ -0,0 +1,91 @@ +# no-duplicate-keyframe-selectors + +Disallow duplicate selectors within keyframe blocks. + +## Background + +The @keyframes at-rule in CSS defines intermediate steps in an animation sequence. Each keyframe selector (like `0%`, `50%`, `100%`, `from`, or `to`) represents a point in the animation timeline and contains styles to apply at that point. + +```css +@keyframes test { + 0% { + opacity: 0; + } + + 100% { + opacity: 1; + } +} +``` + +If a selector is repeated within the same @keyframes block, the last declaration wins, potentially causing unintentional overrides or confusion. + +## Rule Details + +This rule warns when it finds a keyframe block that contains duplicate selectors. + +Examples of **incorrect** code for this rule: + +```css +@keyframes test { + 0% { + opacity: 0; + } + + 0% { + opacity: 1; + } +} + +@keyframes test { + from { + opacity: 0; + } + + from { + opacity: 1; + } +} + +@keyframes test { + from { + opacity: 0; + } + + from { + opacity: 1; + } +} +``` + +Examples of **correct** code for this rule: + +```css +@keyframes test { + 0% { + opacity: 0; + } + + 100% { + opacity: 1; + } +} + +@keyframes test { + from { + opacity: 0; + } + + to { + opacity: 1; + } +} +``` + +## When Not to Use It + +If you aren't concerned with duplicate selectors within keyframe blocks, you can safely disable this rule. + +## Prior Art + +- [`keyframe-block-no-duplicate-selectors`](https://stylelint.io/user-guide/rules/keyframe-block-no-duplicate-selectors/) diff --git a/eslint.config.js b/eslint.config.js index ab987c2..34c7b0f 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -11,6 +11,7 @@ import eslintConfigESLint from "eslint-config-eslint"; import eslintPlugin from "eslint-plugin-eslint-plugin"; import json from "@eslint/json"; import { defineConfig, globalIgnores } from "eslint/config"; +import css from "./src/index.js"; //----------------------------------------------------------------------------- // Helpers @@ -30,7 +31,7 @@ const eslintPluginTestsRecommendedConfig = //----------------------------------------------------------------------------- export default defineConfig([ - globalIgnores(["**/tests/fixtures/", "**/dist/"]), + globalIgnores(["**/tests/fixtures/", "**/dist/", "test.css"]), ...eslintConfigESLint.map(config => ({ files: ["**/*.js"], @@ -115,4 +116,9 @@ export default defineConfig([ "eslint-plugin/test-case-shorthand-strings": "error", }, }, + { + files: ["**/*.css"], + language: "css/css", + ...css.configs.recommended, + }, ]); diff --git a/src/index.js b/src/index.js index f6a30b1..3e70179 100644 --- a/src/index.js +++ b/src/index.js @@ -11,6 +11,7 @@ import { CSSLanguage } from "./languages/css-language.js"; import { CSSSourceCode } from "./languages/css-source-code.js"; import noEmptyBlocks from "./rules/no-empty-blocks.js"; import noDuplicateImports from "./rules/no-duplicate-imports.js"; +import noDuplicateKeyframeSelectors from "./rules/no-duplicate-keyframe-selectors.js"; import noImportant from "./rules/no-important.js"; import noInvalidProperties from "./rules/no-invalid-properties.js"; import noInvalidAtRules from "./rules/no-invalid-at-rules.js"; @@ -34,6 +35,7 @@ const plugin = { rules: { "no-empty-blocks": noEmptyBlocks, "no-duplicate-imports": noDuplicateImports, + "no-duplicate-keyframe-selectors": noDuplicateKeyframeSelectors, "no-important": noImportant, "no-invalid-at-rules": noInvalidAtRules, "no-invalid-properties": noInvalidProperties, @@ -50,6 +52,7 @@ const plugin = { "css/no-duplicate-imports": "error", "css/no-important": "error", "css/no-invalid-at-rules": "error", + "css/no-duplicate-keyframe-selectors": "error", "css/no-invalid-properties": "error", "css/use-baseline": "warn", }), diff --git a/src/rules/no-duplicate-keyframe-selectors.js b/src/rules/no-duplicate-keyframe-selectors.js new file mode 100644 index 0000000..a70e79f --- /dev/null +++ b/src/rules/no-duplicate-keyframe-selectors.js @@ -0,0 +1,73 @@ +/** + * @fileoverview Rule to disallow duplicate selectors within keyframe blocks. + * @author Nitin Kumar + */ + +//----------------------------------------------------------------------------- +// Type Definitions +//----------------------------------------------------------------------------- + +/** + * @import { CSSRuleDefinition } from "../types.js" + * @typedef {"duplicateKeyframeSelector"} DuplicateKeyframeSelectorMessageIds + * @typedef {CSSRuleDefinition<{ RuleOptions: [], MessageIds: DuplicateKeyframeSelectorMessageIds }>} DuplicateKeyframeSelectorRuleDefinition + */ + +//----------------------------------------------------------------------------- +// Rule Definition +//----------------------------------------------------------------------------- + +/** @type {DuplicateKeyframeSelectorRuleDefinition} */ +export default { + meta: { + type: "problem", + + docs: { + description: "Disallow duplicate selectors within keyframe blocks", + recommended: true, + url: "https://github.com/eslint/css/blob/main/docs/rules/no-duplicate-keyframe-selectors.md", + }, + + messages: { + duplicateKeyframeSelector: + "Unexpected duplicate selector '{{selector}}' found within keyframe block.", + }, + }, + + create(context) { + return { + Atrule(node) { + if (node.name === "keyframes" && node.block) { + const selectorNodes = node.block.children.map(child => { + // @ts-ignore - prelude is a valid property + const selector = child.prelude.children[0].children[0]; + let value; + if (selector.type === "Percentage") { + value = `${selector.value}%`; + } else if (selector.type === "TypeSelector") { + value = selector.name.toLowerCase(); + } else { + value = selector.value; + } + return { value, loc: selector.loc }; + }); + + const seen = new Map(); + selectorNodes.forEach((selectorNode, index) => { + if (seen.has(selectorNode.value)) { + context.report({ + loc: selectorNode.loc, + messageId: "duplicateKeyframeSelector", + data: { + selector: selectorNode.value, + }, + }); + } else { + seen.set(selectorNode.value, index); + } + }); + } + }, + }; + }, +}; diff --git a/tests/rules/no-duplicate-keyframe-selectors.test.js b/tests/rules/no-duplicate-keyframe-selectors.test.js new file mode 100644 index 0000000..504aafa --- /dev/null +++ b/tests/rules/no-duplicate-keyframe-selectors.test.js @@ -0,0 +1,249 @@ +/** + * @fileoverview Tests for no-duplicate-keyframe-selectors rule. + * @author Nitin Kumar + */ + +//------------------------------------------------------------------------------ +// Imports +//------------------------------------------------------------------------------ + +import rule from "../../src/rules/no-duplicate-keyframe-selectors.js"; +import css from "../../src/index.js"; +import { RuleTester } from "eslint"; +import dedent from "dedent"; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + plugins: { + css, + }, + language: "css/css", +}); + +ruleTester.run("no-duplicate-keyframe-selectors", rule, { + valid: [ + dedent`@keyframes test { + from { opacity: 0; } + to { opacity: 1; } + }`, + dedent`@keyframes test { + 0% { opacity: 0; } + 100% { opacity: 1; } + }`, + dedent`@keyframes test { + 0% { opacity: 0; } + 50% { opacity: 0.5; } + 100% { opacity: 1; } + }`, + dedent`@keyframes test { + from { opacity: 0; } + 50% { opacity: 0.5; } + to { opacity: 1; } + }`, + dedent`@keyframes test { + }`, + dedent`@keyframes test { + 0% { opacity: 0; } + }`, + dedent`@keyframes test { + 0% { opacity: 0; } + 0.0% { opacity: 1; } + }`, + ], + invalid: [ + { + code: dedent`@keyframes test { + 0% { opacity: 0; } + 0% { opacity: 1; } + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 3, + column: 5, + endLine: 3, + endColumn: 7, + }, + ], + }, + { + code: dedent`@keyframes test { + 0% { opacity: 0; } + 10.5% { opacity: 0.15; } + 10.5% { opacity: 0.25; } + 100% { opacity: 1; } + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 4, + column: 5, + endLine: 4, + endColumn: 10, + }, + ], + }, + { + code: dedent`@keyframes test { + from { opacity: 0; } + from { opacity: 1; } + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 3, + column: 5, + endLine: 3, + endColumn: 9, + }, + ], + }, + { + code: dedent`@keyframes test { + from { opacity: 0; } + From { opacity: 1; } + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 3, + column: 5, + endLine: 3, + endColumn: 9, + }, + ], + }, + { + code: dedent`@keyframes test { + from { opacity: 0; } + FROM { opacity: 1; } + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 3, + column: 5, + endLine: 3, + endColumn: 9, + }, + ], + }, + { + code: dedent`@keyframes test { + from { opacity: 0; } + to { opacity: 1; } + to { opacity: 2; } + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 4, + column: 5, + endLine: 4, + endColumn: 7, + }, + ], + }, + { + code: dedent`@keyframes test { + from { opacity: 0; } + to { opacity: 1; } + TO { opacity: 2; } + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 4, + column: 5, + endLine: 4, + endColumn: 7, + }, + ], + }, + { + code: dedent`@keyframes test { + 0% { opacity: 0; } + 50% { opacity: 0.5; } + 50% { opacity: 0.75; } + 100% { opacity: 1; } + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 4, + column: 5, + endLine: 4, + endColumn: 8, + }, + ], + }, + { + code: dedent`@keyframes test { + 0% { + opacity: 0; + } + + 0% { + opacity: 1; + } + + 50% { + opacity: 0.5; + } + + 50% { + opacity: 0.75; + } + + 50% { + opacity: 0.5; + } + + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 6, + column: 5, + endLine: 6, + endColumn: 7, + }, + { + messageId: "duplicateKeyframeSelector", + line: 14, + column: 5, + endLine: 14, + endColumn: 8, + }, + { + messageId: "duplicateKeyframeSelector", + line: 18, + column: 5, + endLine: 18, + endColumn: 8, + }, + ], + }, + { + code: dedent`@keyframes test { + /* Start */ + 0% { opacity: 0; } + /* Middle */ + 0% { opacity: 1; } + /* End */ + }`, + errors: [ + { + messageId: "duplicateKeyframeSelector", + line: 5, + column: 5, + endLine: 5, + endColumn: 7, + }, + ], + }, + ], +}); From 470e3a9ff923437d6f4b0c66654deced905e5570 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Thu, 22 May 2025 00:33:07 +0530 Subject: [PATCH 2/7] docs: update --- docs/rules/no-duplicate-keyframe-selectors.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-duplicate-keyframe-selectors.md b/docs/rules/no-duplicate-keyframe-selectors.md index 2a9aaad..bfe3f14 100644 --- a/docs/rules/no-duplicate-keyframe-selectors.md +++ b/docs/rules/no-duplicate-keyframe-selectors.md @@ -4,7 +4,7 @@ Disallow duplicate selectors within keyframe blocks. ## Background -The @keyframes at-rule in CSS defines intermediate steps in an animation sequence. Each keyframe selector (like `0%`, `50%`, `100%`, `from`, or `to`) represents a point in the animation timeline and contains styles to apply at that point. +The [`@keyframes` at-rule](https://developer.mozilla.org/en-US/docs/Web/CSS/@keyframes) in CSS defines intermediate steps in an animation sequence. Each keyframe selector (like `0%`, `50%`, `100%`, `from`, or `to`) represents a point in the animation timeline and contains styles to apply at that point. ```css @keyframes test { @@ -27,6 +27,8 @@ This rule warns when it finds a keyframe block that contains duplicate selectors Examples of **incorrect** code for this rule: ```css +/* eslint css/no-duplicate-keyframe-selectors: "error" */ + @keyframes test { 0% { opacity: 0; @@ -61,6 +63,8 @@ Examples of **incorrect** code for this rule: Examples of **correct** code for this rule: ```css +/* eslint css/no-duplicate-keyframe-selectors: "error" */ + @keyframes test { 0% { opacity: 0; From ae83968e43b8b725cccb54b0f0e27a8432199420 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sat, 31 May 2025 23:10:43 +0530 Subject: [PATCH 3/7] chore: refactor --- src/rules/no-duplicate-keyframe-selectors.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rules/no-duplicate-keyframe-selectors.js b/src/rules/no-duplicate-keyframe-selectors.js index a70e79f..52b1b34 100644 --- a/src/rules/no-duplicate-keyframe-selectors.js +++ b/src/rules/no-duplicate-keyframe-selectors.js @@ -39,8 +39,9 @@ export default { Atrule(node) { if (node.name === "keyframes" && node.block) { const selectorNodes = node.block.children.map(child => { - // @ts-ignore - prelude is a valid property - const selector = child.prelude.children[0].children[0]; + // eslint-disable-next-line dot-notation -- bracket notation to avoid type error even though it's valid + const selector = + child["prelude"].children[0].children[0]; let value; if (selector.type === "Percentage") { value = `${selector.value}%`; From 53d4a681eb2f3b02195703cec036fbb285011854 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Sat, 31 May 2025 23:11:04 +0530 Subject: [PATCH 4/7] chore: refactor --- src/rules/no-duplicate-keyframe-selectors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-duplicate-keyframe-selectors.js b/src/rules/no-duplicate-keyframe-selectors.js index 52b1b34..00a0d49 100644 --- a/src/rules/no-duplicate-keyframe-selectors.js +++ b/src/rules/no-duplicate-keyframe-selectors.js @@ -39,8 +39,8 @@ export default { Atrule(node) { if (node.name === "keyframes" && node.block) { const selectorNodes = node.block.children.map(child => { - // eslint-disable-next-line dot-notation -- bracket notation to avoid type error even though it's valid const selector = + // eslint-disable-next-line dot-notation -- bracket notation to avoid type error even though it's valid child["prelude"].children[0].children[0]; let value; if (selector.type === "Percentage") { From 864aeb2c9733901b8abb020a69807188e8d354f9 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Fri, 18 Jul 2025 10:56:32 +0530 Subject: [PATCH 5/7] chore: update eslint config for css files --- eslint.config.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eslint.config.js b/eslint.config.js index 9dd855b..c9d9e58 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -119,7 +119,8 @@ export default defineConfig([ { files: ["**/*.css"], language: "css/css", - ...css.configs.recommended, + plugins: { css }, + extends: ["css/recommended"], }, { files: ["tools/**/*.js"], From 6db3bf410bf6034dc569190d2b9bf0eefceb1117 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Fri, 18 Jul 2025 11:04:13 +0530 Subject: [PATCH 6/7] refactor: rule logic --- src/rules/no-duplicate-keyframe-selectors.js | 63 ++++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/rules/no-duplicate-keyframe-selectors.js b/src/rules/no-duplicate-keyframe-selectors.js index 00a0d49..1559423 100644 --- a/src/rules/no-duplicate-keyframe-selectors.js +++ b/src/rules/no-duplicate-keyframe-selectors.js @@ -35,39 +35,38 @@ export default { }, create(context) { + let selectorNodes = []; return { - Atrule(node) { - if (node.name === "keyframes" && node.block) { - const selectorNodes = node.block.children.map(child => { - const selector = - // eslint-disable-next-line dot-notation -- bracket notation to avoid type error even though it's valid - child["prelude"].children[0].children[0]; - let value; - if (selector.type === "Percentage") { - value = `${selector.value}%`; - } else if (selector.type === "TypeSelector") { - value = selector.name.toLowerCase(); - } else { - value = selector.value; - } - return { value, loc: selector.loc }; - }); - - const seen = new Map(); - selectorNodes.forEach((selectorNode, index) => { - if (seen.has(selectorNode.value)) { - context.report({ - loc: selectorNode.loc, - messageId: "duplicateKeyframeSelector", - data: { - selector: selectorNode.value, - }, - }); - } else { - seen.set(selectorNode.value, index); - } - }); - } + "Atrule[name='keyframes']"(node) { + selectorNodes = node.block.children.map(child => { + const selector = child.prelude.children[0].children[0]; + let value; + if (selector.type === "Percentage") { + value = `${selector.value}%`; + } else if (selector.type === "TypeSelector") { + value = selector.name.toLowerCase(); + } else { + value = selector.value; + } + return { value, loc: selector.loc }; + }); + }, + "Atrule[name=keyframes]:exit"() { + const seen = new Map(); + selectorNodes.forEach((selectorNode, index) => { + if (seen.has(selectorNode.value)) { + context.report({ + loc: selectorNode.loc, + messageId: "duplicateKeyframeSelector", + data: { + selector: selectorNode.value, + }, + }); + } else { + seen.set(selectorNode.value, index); + } + }); + selectorNodes = []; }, }; }, From 831fac013b2e85f890b6f23a0250d05fd0876361 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Fri, 18 Jul 2025 11:12:16 +0530 Subject: [PATCH 7/7] refactor: improve traversing logic --- src/rules/no-duplicate-keyframe-selectors.js | 66 +++++++++++--------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/src/rules/no-duplicate-keyframe-selectors.js b/src/rules/no-duplicate-keyframe-selectors.js index 1559423..da97d74 100644 --- a/src/rules/no-duplicate-keyframe-selectors.js +++ b/src/rules/no-duplicate-keyframe-selectors.js @@ -35,38 +35,46 @@ export default { }, create(context) { - let selectorNodes = []; + let insideKeyframes = false; + const seen = new Map(); + return { - "Atrule[name='keyframes']"(node) { - selectorNodes = node.block.children.map(child => { - const selector = child.prelude.children[0].children[0]; - let value; - if (selector.type === "Percentage") { - value = `${selector.value}%`; - } else if (selector.type === "TypeSelector") { - value = selector.name.toLowerCase(); - } else { - value = selector.value; - } - return { value, loc: selector.loc }; - }); + "Atrule[name=keyframes]"() { + insideKeyframes = true; + seen.clear(); }, + "Atrule[name=keyframes]:exit"() { - const seen = new Map(); - selectorNodes.forEach((selectorNode, index) => { - if (seen.has(selectorNode.value)) { - context.report({ - loc: selectorNode.loc, - messageId: "duplicateKeyframeSelector", - data: { - selector: selectorNode.value, - }, - }); - } else { - seen.set(selectorNode.value, index); - } - }); - selectorNodes = []; + insideKeyframes = false; + }, + + Rule(node) { + if (!insideKeyframes) { + return; + } + + // @ts-ignore - children is a valid property for prelude + const selector = node.prelude.children[0].children[0]; + let value; + if (selector.type === "Percentage") { + value = `${selector.value}%`; + } else if (selector.type === "TypeSelector") { + value = selector.name.toLowerCase(); + } else { + value = selector.value; + } + + if (seen.has(value)) { + context.report({ + loc: selector.loc, + messageId: "duplicateKeyframeSelector", + data: { + selector: value, + }, + }); + } else { + seen.set(value, true); + } }, }; },