Skip to content

fix: compare percentages by value in no-duplicate-keyframe-selectors - #604

Open
xbinaryx wants to merge 1 commit into
eslint:mainfrom
xbinaryx:no-duplicate-keyframe-selectors-numeric-values
Open

xbinaryx wants to merge 1 commit into
eslint:mainfrom
xbinaryx:no-duplicate-keyframe-selectors-numeric-values

Conversation

@xbinaryx

@xbinaryx xbinaryx commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Prerequisites checklist

AI acknowledgment

  • I did not use AI to generate this PR.
  • (If the above is not checked) I have reviewed the AI-generated content before submitting.

What did you do?

/* eslint css/no-duplicate-keyframe-selectors: "error" */

@keyframes fade {
	0% {
		opacity: 0;
	}

	0.0% {
		opacity: 1;
	}
}

What did you expect to happen?

0.0% is reported as a duplicate of 0%.

What actually happened?

No problems were reported.

What is the purpose of this pull request?

Different spellings of the same percentage, like 0% and 0.0%, select the same keyframe, so the later block overrides the earlier one. The rule compared them as text and didn't report them.

What changes did you make? (Give an overview)

  • Compared percentages by Number(component.value) instead of their source text.
  • Documented the behavior in the rule docs.

Related Issues

Is there anything you'd like reviewers to focus on?

Summary by CodeRabbit

  • Bug Fixes
    • Duplicate keyframe selectors are now detected when percentage values are numerically equivalent, even if written differently—for example, 0% and 0.0%. This also applies to equivalent forms used with from, to, comma-separated selectors, and prefixed entry selectors.
    • Type-selector names are now compared without regard to letter case when checking from and to aliases. Reported selector text retains its original spelling.
  • Documentation
    • Updated the rule documentation with the numeric comparison behavior and an example of duplicate percentage selectors.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Triage Oct 30, 2026
@eslint-github-bot eslint-github-bot Bot added the bug Something isn't working label Oct 30, 2026
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Central YAML; Path: .coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b1b2ef48-c5f9-43eb-b811-69bdd77bb210
Commits

Reviewing files that changed from the base of the PR and between 61ba541 and e9dfd12.

Files selected for processing: 3
  • docs/rules/no-duplicate-keyframe-selectors.md
  • src/rules/no-duplicate-keyframe-selectors.js
  • tests/rules/no-duplicate-keyframe-selectors.test.js

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

The rule now detects duplicate keyframe selectors when percentage components have equal numeric values. Tests cover equivalent percentage spellings and aliases. The documentation describes the comparison and includes an incorrect-code example.

Changes: Keyframe selector comparison

Layer / File(s) Summary
Normalize comparisons and validate equivalent selectors
src/rules/no-duplicate-keyframe-selectors.js, tests/rules/no-duplicate-keyframe-selectors.test.js, docs/rules/no-duplicate-keyframe-selectors.md
The rule normalizes percentage values for duplicate comparisons and retains the original selector values for reports. Tests cover equivalent numeric forms, from/to aliases, comma-separated selectors, and entry selectors. The valid fixture and rule documentation reflect the behavior.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested labels: bug

Suggested reviewers: pixel998

Merge Risk: Minimal · up to e9dfd

The change detects numerically equivalent percentage selectors and adds corresponding tests and documentation. No concrete merge-blocking risk is identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: comparing percentage selectors by numeric value in the specified rule.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

},
],
},
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please also add a test case with a trailing ".", for example:
1% and 1.%?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.% isn't a valid percentage, so it's a parsing error.

// @ts-ignore - children is a valid property for prelude
node.prelude.children.forEach(selector => {
const value = [];
const keyParts = [];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of keyParts?
Why could we not push the normalized value directly into the value array?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value builds the error message, which shows the selector as written ('0.0%', 'From'). keyParts holds the normalized form used for the duplicate check. Pushing the normalized value into value would make the message for 0.0% say '0%'.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a short comment?

@DMartens DMartens moved this from Needs Triage to Implementing in Triage Oct 30, 2026
@DMartens DMartens added the accepted There is consensus among the team that this change meets the criteria for inclusion label Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion bug Something isn't working

Projects

Status: Implementing

Development

Successfully merging this pull request may close these issues.

3 participants