Skip to content

Conversation

@lumirlumir
Copy link
Member

Prerequisites checklist

What is the purpose of this pull request?

In this PR, I've updated the no-unnormlized-keys rule to use defaultOptions.

Previously, the rule used custom logic to extract its default value.

According to MDN and @types/node, the default value for String.prototype.normalize is "NFC", so I've set it as the default.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/normalize#parameters

image image

What changes did you make? (Give an overview)

I've updated the no-unnormlized-keys rule to use defaultOptions.

Related Issues

N/A

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

N/A

@eslintbot eslintbot added this to Triage Sep 13, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Sep 13, 2025
@lumirlumir lumirlumir requested a review from a team September 13, 2025 06:13
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit 80bc62e into main Sep 13, 2025
23 checks passed
@github-project-automation github-project-automation bot moved this from Needs Triage to Complete in Triage Sep 13, 2025
@mdjermanovic mdjermanovic deleted the refactor-use-default-options-in-no-unnormalized-keys branch September 13, 2025 13:28
@aladdin-add
Copy link
Member

would it be a breaking change? the doc says, "This plugin requires ESLint v9.6.0 or higher", but meta.defaultOptions was added in v9.15.0: https://github.com/eslint/eslint/releases/tag/v9.15.0

@lumirlumir
Copy link
Member Author

lumirlumir commented Sep 13, 2025

Hmm, I'm not sure about this 🤔

But since the sort-keys rule is already using defaultOptions as shown below, and @eslint/json is still in the 0.X unstable version, I think it would be safe. Also, we don't have peerDependencies in the package.json specifying the relevant eslint version, so maybe it's fine?

defaultOptions: [
"asc",
{
allowLineSeparatedGroups: false,
caseSensitive: true,
minKeys: 2,
natural: false,
},
],

Or maybe just updating the documentation would be the best approach here?

@aladdin-add
Copy link
Member

IMHO, the safest approach is to avoid using defaultOptions - we can update these in next major (dropping eslint <9.15).

@mdjermanovic
Copy link
Member

would it be a breaking change? the doc says, "This plugin requires ESLint v9.6.0 or higher", but meta.defaultOptions was added in v9.15.0: https://github.com/eslint/eslint/releases/tag/v9.15.0

Good catch!

Well, since this plugin is technically incompatible with eslint < 9.15 since v0.10.0 when we added the json/sort-keys rule with implementation that needs the defaultOption feature to work properly, maybe we could just update the docs now. @eslint/eslint-tsc thoughts?

@fasttime
Copy link
Member

maybe we could just update the docs now.

Yes, at a minimum the docs should be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

5 participants