Skip to content

fix: preserve keyframe selector casing in error messages - #571

Merged
DMartens merged 3 commits into
eslint:mainfrom
KumJungMin:fix/issue-562
Sep 21, 2026
Merged

DMartens merged 3 commits into
eslint:mainfrom
KumJungMin:fix/issue-562

Conversation

@KumJungMin

@KumJungMin KumJungMin commented Sep 19, 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 is the purpose of this pull request?

  • Preserve the original casing of selectors in no-duplicate-keyframe-selectors error messages.
  • For example, a duplicate TO is reported as TO instead of to.

What changes did you make? (Give an overview)

  • Separate the values used for duplicate detection from those displayed in error messages.
  • Update six existing test expectations to reflect the reported selector's original casing.
  • Duplicate detection, error locations, and whitespace and comment handling remain unchanged.

Related Issues

fixes #562

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


Disclosure: I'm a participant of open source contribution program OSSCA

Summary by CodeRabbit

  • Bug Fixes
    • Duplicate keyframe selector errors now display selectors with their original type-selector casing.
    • Selector comparison remains case-insensitive, while percentage formatting stays normalized with %.
  • Tests
    • Updated validation coverage to confirm duplicate selectors are reported using their source spelling.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 65313696-5096-44eb-a47d-921071a14730

📥 Commits

Reviewing files that changed from the base of the PR and between d0b26a4 and 963d3dc.

📒 Files selected for processing (1)
  • src/rules/no-duplicate-keyframe-selectors.js

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The rule continues case-insensitive duplicate detection and now reports duplicate selectors with their original casing. Tests update expected diagnostic data for mixed-case selectors.

Changes

Keyframe selector diagnostics

Layer / File(s) Summary
Separate detection and diagnostic selectors
src/rules/no-duplicate-keyframe-selectors.js, tests/rules/no-duplicate-keyframe-selectors.test.js
The rule keeps normalized selectors for comparison and preserves original casing for diagnostics. Tests expect the source casing for duplicate from, to, and entry 0% selectors.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: pixel998

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #562 requires original selector casing in messages and case-insensitive duplicate detection. The rule keeps normalized values for duplicate keys and reports original type-selector casing. The te…
Out of Scope Changes check ✅ Passed The changes are limited to the rule implementation and its tests. The implementation separates comparison values from displayed selector text. The test updates verify the requested message casing. No …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving keyframe selector casing in error messages.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

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

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.

I do not think we need the variable rawValue.
We can just push the the values as we do currently and use the lowercase variant when computing key in the map callback (line 78).

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.

Thank you for the comment.
I hadn’t considered that keeping a separate rawValue array would mean maintaining the same data in two places.
I’ve updated the code as follows :)

963d3dc

@DMartens DMartens moved this from Needs Triage to Implementing in Triage Sep 19, 2026
@DMartens DMartens added the accepted There is consensus among the team that this change meets the criteria for inclusion label Sep 19, 2026
@KumJungMin
KumJungMin requested a review from DMartens September 20, 2026 23:16

@DMartens DMartens left a comment

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.

Changes LGTM, thanks.

@DMartens DMartens changed the title feat: preserve keyframe selector casing in error messages fix: preserve keyframe selector casing in error messages Sep 21, 2026
@eslint-github-bot eslint-github-bot Bot added the bug Something isn't working label Sep 21, 2026
@DMartens
DMartens merged commit 6589273 into eslint:main Sep 21, 2026
39 checks passed
@github-project-automation github-project-automation Bot moved this from Implementing to Complete in Triage Sep 21, 2026
@KumJungMin
KumJungMin deleted the fix/issue-562 branch September 29, 2026 03:16
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 feature

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Change Request: Preserve the original casing of selectors in no-duplicate-keyframe-selectors error messages

2 participants