Skip to content

Conversation

lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Sep 14, 2025

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

In this PR, I've updated eslint-all.js to use Object.freeze for the rules object.

I noticed it was odd that the all configuration didn't use Object.freeze, especially since the recommended configuration does, and the TypeScript type signature indicates that the rules object is readonly.

module.exports = Object.freeze({
rules: Object.freeze({
"constructor-super": "error",
"for-direction": "error",

readonly all: { readonly rules: Readonly<Linter.RulesRecord> };

So, to keep things consistent across the rule configurations and with the type signature, I've updated eslint-all.js to use Object.freeze for the rules object.

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

I'm not sure if this should be considered a breaking change. If anyone has another opinion, I'm happy to follow that suggestion.

@lumirlumir lumirlumir requested a review from a team as a code owner September 14, 2025 09:41
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Sep 14, 2025
@eslint-github-bot eslint-github-bot bot added the bug ESLint is working incorrectly label Sep 14, 2025
Copy link

netlify bot commented Sep 14, 2025

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 6196ede
🔍 Latest deploy log https://app.netlify.com/projects/docs-eslint/deploys/68c68de4ee1a15000803d97e

@lumirlumir lumirlumir changed the title fix: update eslint-all.js to use Object.freeze for rules object fix: update eslint-all.js to use Object.freeze for rules object Sep 14, 2025
@mdjermanovic
Copy link
Member

I'm not sure if this should be considered a breaking change. If anyone has another opinion, I'm happy to follow that suggestion.

I think we can treat this as a non-breaking change. @eslint/eslint-tsc thoughts?

@mdjermanovic mdjermanovic moved this from Needs Triage to Feedback Needed in Triage Sep 14, 2025
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.

Changes LGTM. Pending opinions on whether this is a breaking change.

@mdjermanovic mdjermanovic added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion core Relates to ESLint's core APIs and features labels Sep 14, 2025
@fasttime
Copy link
Member

I think we can treat this as a non-breaking change.

I agree this isn't a breaking change.

@nzakas
Copy link
Member

nzakas commented Sep 15, 2025

Agreed.

@nzakas nzakas merged commit 1c0d850 into main Sep 15, 2025
30 checks passed
@nzakas nzakas deleted the fix-update-eslint-all-to-use-object-freeze-for-rules-object branch September 15, 2025 15:08
@github-project-automation github-project-automation bot moved this from Feedback Needed to Complete in Triage Sep 15, 2025
nzakas pushed a commit that referenced this pull request Sep 23, 2025
…#20116)

fix: update `eslint-all.js` to use `Object.freeze` for rules object
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ESLint is working incorrectly core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

4 participants