-
Notifications
You must be signed in to change notification settings - Fork 194
chore(eslint): use flat configuration #851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This will allow to upgrade eslint to v9.
WalkthroughThis change removes the old ESLint configuration files and introduces a new modular ESLint setup using Changes
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eslint.config.mjs (1)
54-58
: Consider implementing the recommended import plugin configuration.The TODO comment indicates plans to add
importPlugin.flatConfigs.recommended
. This would provide additional import-related rules beyond the basic plugin registration.Consider replacing the current import plugin setup with the recommended configuration:
- // TODO configure import with more rules by adding "importPlugin.flatConfigs.recommended" - { - plugins: { - import: importPlugin, // may no longer be needed whe loading recommended config of import plugin - }, + importPlugin.flatConfigs.recommended, + {This would provide better import validation while maintaining the custom rules below.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (5)
.eslintignore
(0 hunks).eslintrc.json
(0 hunks).github/dependabot.yml
(1 hunks)eslint.config.mjs
(1 hunks)package.json
(1 hunks)
💤 Files with no reviewable changes (2)
- .eslintignore
- .eslintrc.json
🔇 Additional comments (7)
package.json (1)
29-30
: LGTM! Dependency consolidation aligns with flat config migration.The migration from separate
@typescript-eslint/eslint-plugin
and@typescript-eslint/parser
packages to the consolidatedtypescript-eslint
package is the correct approach for ESLint flat configuration. This consolidation simplifies dependency management while maintaining full functionality..github/dependabot.yml (1)
46-49
: LGTM! Dependabot patterns updated to match new package structure.The pattern updates correctly reflect the dependency changes in
package.json
. Replacing@typescript-eslint/*
witheslint
and addingtypescript-eslint
ensures proper grouping of lint-related dependencies under the new consolidated package structure.eslint.config.mjs (5)
1-21
: LGTM! Proper license header and modern imports.The Apache 2.0 license header is correctly included, and the imports use the modern flat config approach with appropriate packages:
@eslint/js
for base configurationeslint-plugin-import
for import ruleseslint-plugin-prettier/recommended
for Prettier integrationtypescript-eslint
for the consolidated TypeScript ESLint package
24-36
: LGTM! Comprehensive ignore patterns replace .eslintignore.The ignore patterns appropriately cover:
- IDE folders (
.github
,.idea
,.vscode
)- Build outputs (
dist
,lib
,build
)- Generated files and dependencies (
node_modules
,coverage
,generated
)This effectively replaces the functionality of the removed
.eslintignore
file.
42-49
: LGTM! Proper JavaScript configuration with type-checking disabled.The JavaScript-specific configuration correctly:
- Targets JS files with appropriate extensions
- Disables TypeScript type-checking for JS files using
tsEslint.configs.disableTypeChecked
- Sets ECMAScript 2020 parsing with module support
- Uses modern language options structure
60-89
: LGTM! Well-balanced custom rule configuration.The custom rules strike a good balance:
- Appropriate warnings:
no-misleading-character-class
,no-dupe-else-if
- Sensible disables:
no-warning-comments
(reasonable for active development), various TypeScript strict rules that may be too restrictive- Important restrictions:
no-console
,no-eval
, const enum prohibition for interoperability- Practical allowances: Short-circuit and ternary expressions in
no-unused-expressions
The const enum restriction with a clear message is particularly well-implemented for maintaining JavaScript interoperability.
92-93
: LGTM! Prettier integration correctly positioned last.The Prettier recommended configuration is correctly positioned as the final configuration to avoid conflicts with other formatting rules. The comment clearly explains the importance of this ordering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eslint.config.mjs (1)
54-58
: Address the TODO for import plugin configuration.The TODO comment indicates incomplete import plugin setup. Consider either implementing the recommended import plugin configuration or creating a follow-up issue to address this.
Would you like me to generate the recommended import plugin configuration or open an issue to track this enhancement?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (5)
.eslintignore
(0 hunks).eslintrc.json
(0 hunks).github/dependabot.yml
(1 hunks)eslint.config.mjs
(1 hunks)package.json
(1 hunks)
💤 Files with no reviewable changes (2)
- .eslintignore
- .eslintrc.json
🔇 Additional comments (7)
package.json (2)
29-30
: LGTM! Dependency consolidation aligns with flat config.The migration from separate
@typescript-eslint/eslint-plugin
and@typescript-eslint/parser
packages to the unifiedtypescript-eslint
package is correct for the flat configuration format.
24-24
: Verify ESLint version alignment with PR objectives.The PR objectives mention that this change enables upgrading ESLint to version 9, but
package.json
still specifies ESLint~8.57.1
. Please confirm if the ESLint 9 upgrade is intended as part of this PR or in a subsequent change.#!/bin/bash # Check if ESLint 9 is compatible with the current flat config setup npm view eslint versions --json | jq '.[-5:]'eslint.config.mjs (4)
17-20
: LGTM! Imports correctly set up for flat config.The imports properly utilize the new unified
typescript-eslint
package and follow flat config patterns.
78-86
: LGTM! Custom const enum restriction is well-implemented.The custom rule to forbid const enums with a clear message promotes better interoperability. This is a good practice for library code.
92-92
: LGTM! Prettier integration correctly positioned.Placing the Prettier configuration last ensures it properly overrides conflicting rules, which is the recommended approach.
24-36
: Let's broaden the search scope to catch any deeper build or temp directories:#!/bin/bash # Remove depth limit to find all matching directories across the repo fd -t d . | grep -E "(build|dist|coverage|tmp|temp|cache)" | head -20.github/dependabot.yml (1)
46-49
: LGTM! Dependabot patterns updated correctly.The lint group patterns properly reflect the new dependency structure, replacing the wildcard
@typescript-eslint/*
pattern with explicit entries foreslint
andtypescript-eslint
. This ensures proper grouping of related linting dependency updates.
This will allow to upgrade eslint to v9.
Summary by CodeRabbit