Migrating from eslintrc.json to eslint.config.js #2660
Conversation
WalkthroughThis pull request focuses on migrating the ESLint configuration from the traditional Changes
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
eslint.config.mjs (2)
7-9: Simplify plugin import by removing 'fixupPluginRules'.The use of
fixupPluginRulesfrom@eslint/compatmay not be necessary in the Flat Configuration. You can importeslint-plugin-importdirectly and reference it in thepluginssection without wrapping it.Apply this diff to simplify the import:
- import _import from "eslint-plugin-import"; - import { fixupPluginRules } from "@eslint/compat"; + import importPlugin from "eslint-plugin-import"; ... plugins: { react, "@typescript-eslint": typescriptEslint, jest, - import: fixupPluginRules(_import), + import: importPlugin, tsdoc, prettier, },
61-62: Update 'ecmaVersion' to the latest version.The
ecmaVersionis set to2018. Consider updating it to the latest version (e.g.,2023) to support the newest ECMAScript features.Apply this diff to update the
ecmaVersion:parser: tsParser, - ecmaVersion: 2018, + ecmaVersion: "latest", sourceType: "module",package.json (1)
Line range hint
97-101: Remove the redundant 'eslintConfig' field from 'package.json'.Since the ESLint configuration has moved to
eslint.config.mjs, theeslintConfigfield inpackage.jsonis no longer needed and can be removed to prevent confusion.Apply this diff to remove the redundant configuration:
}, - "eslintConfig": { - "extends": [ - "react-app", - "react-app/jest" - ] - }, "browserslist": {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.eslintignore(0 hunks).eslintrc.json(0 hunks)eslint.config.mjs(1 hunks)package.json(2 hunks)src/GraphQl/Queries/fundQueries.ts(0 hunks)src/screens/OrganizationDashboard/OrganizationDashboard.tsx(0 hunks)
💤 Files with no reviewable changes (4)
- .eslintignore
- .eslintrc.json
- src/screens/OrganizationDashboard/OrganizationDashboard.tsx
- src/GraphQl/Queries/fundQueries.ts
🔇 Additional comments (2)
eslint.config.mjs (1)
24-33: Verify that all necessary files are properly ignored.
With the migration from .eslintignore to the ignores array, ensure that all files and patterns that should be excluded from linting are included. This includes any files previously specified in .eslintignore.
Review the ignores array to confirm it reflects all the necessary ignore patterns from your previous configuration.
package.json (1)
119-121: Re-evaluate the necessity of '@eslint/compat' and '@eslint/eslintrc'.
The packages @eslint/compat and @eslint/eslintrc are typically used for compatibility purposes when migrating configurations. Since you're adopting the new Flat Configuration format, these may not be required.
Consider removing these dependencies if they are not essential to your setup.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
eslint.config.mjs (2)
51-68: Consider updating ECMAScript version.The current configuration uses ECMAScript 2018. Consider updating to a more recent version (e.g., 2022) to take advantage of newer language features.
- ecmaVersion: 2018, + ecmaVersion: 2022,
112-116: Consider relaxing interface naming conventions.The current configuration requires all interfaces to have either "Interface" or "TestInterface" prefix, which might lead to verbose and less readable code. Consider removing the prefix requirement while keeping the PascalCase format.
"@typescript-eslint/naming-convention": ["error", { selector: "interface", format: ["PascalCase"], - prefix: ["Interface", "TestInterface"], }, {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eslint.config.mjs(1 hunks)
🔇 Additional comments (6)
eslint.config.mjs (6)
1-14: LGTM: Import statements are well-organized and complete.
All necessary ESLint plugins and utilities are properly imported.
15-21: LGTM: Configuration setup follows ESLint flat config best practices.
Proper initialization of FlatCompat with correct base directory and ESM compatibility.
34-41: Remove redundant Prettier configuration.
Having both 'plugin:prettier/recommended' and 'prettier' in the extends array is redundant.
42-49: LGTM: Plugin configurations are properly set up.
All plugins are correctly registered and the use of fixupPluginRules for the import plugin is appropriate.
70-74: LGTM: React settings are properly configured.
Using "detect" for React version is the recommended approach.
23-33: Verify the necessity of ignoring specific files.
While the general ignore patterns for CSS/SCSS/JSON files are standard, let's verify if ignoring specific files like src/components/CheckIn/tagTemplate.ts is still necessary.
✅ Verification successful
Let me gather more information about the tagTemplate.ts file to understand why it might need to be ignored.
Based on the results, I can now generate the final response:
The specific file ignores are still necessary and valid
The specifically ignored file src/components/CheckIn/tagTemplate.ts exists and contains a PDF template definition used for tag generation. The file contains base64-encoded PDF data and specific layout configurations, which should be excluded from linting to avoid unnecessary processing of generated/binary content.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the specific ignored files exist and their content
echo "Checking for existence of specifically ignored files..."
for file in "src/components/CheckIn/tagTemplate.ts" "tsconfig.json"
do
if [ -f "$file" ]; then
echo "Found $file:"
head -n 5 "$file"
else
echo "$file not found"
fi
done
Length of output: 798
Script:
#!/bin/bash
# Check the content and usage of tagTemplate.ts
echo "Content of tagTemplate.ts:"
cat src/components/CheckIn/tagTemplate.ts
echo -e "\nChecking for references to tagTemplate:"
rg "tagTemplate" --type ts -C 2
Length of output: 65728
eslint.config.mjs
Outdated
| "@typescript-eslint/ban-ts-comment": "error", | ||
| "@typescript-eslint/no-unused-vars": "error", | ||
| "@typescript-eslint/no-explicit-any": "error", | ||
| "@typescript-eslint/no-inferrable-types": "error", | ||
| "@typescript-eslint/no-non-null-asserted-optional-chain": "error", | ||
| "@typescript-eslint/no-non-null-assertion": "error", | ||
| "@typescript-eslint/no-var-requires": "error", | ||
| "@typescript-eslint/no-unsafe-function-type": "error", | ||
| "@typescript-eslint/no-wrapper-object-types": "error", |
There was a problem hiding this comment.
💡 Codebase verification
Strict TypeScript rules need adjustments before enabling
The codebase shows multiple violations of the proposed strict TypeScript rules:
- Found 31 instances of
anytype usage across multiple files - Found one
@ts-ignorecomment insrc/utils/recurrenceUtils/recurrenceUtilityFunctions.ts
Key files requiring attention:
src/utils/useLocalstorage.ts: Multipleanytype usages in APIsrc/state/action-creators/index.ts: Multipleanytype usages in action creatorssrc/state/reducers/pluginReducer.ts:anytype used for statesrc/utils/StaticMockLink.ts: Multipleanytype usages in request handlingsrc/components/AddOn/support/services/Plugin.helper.ts:anytype used in plugin handling
The strict TypeScript rules should be enabled gradually after proper type definitions are added to replace the any types. Consider:
- First fixing the type definitions
- Then enabling these rules one at a time
- Or enabling them with warnings first (
"warn"instead of"error")
🔗 Analysis chain
Verify the impact of strict TypeScript rules.
The configuration includes several strict TypeScript rules. Let's verify if the codebase is ready for these strict rules to avoid excessive errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential violations of strict TypeScript rules
echo "Checking for potential 'any' types..."
rg ":\s*any\b" --type=typescript
echo "Checking for non-null assertions..."
rg "[!]\." --type=typescript
echo "Checking for @ts-ignore or @ts-nocheck..."
rg "@ts-(ignore|nocheck)" --type=typescript
Length of output: 481
Script:
#!/bin/bash
# Check for potential violations of strict TypeScript rules in .ts and .tsx files
echo "Checking for potential 'any' types..."
rg ":\s*any\b" -g "*.{ts,tsx}"
echo "Checking for non-null assertions..."
rg "[!]\." -g "*.{ts,tsx}"
echo "Checking for @ts-ignore or @ts-nocheck..."
rg "@ts-(ignore|nocheck)" -g "*.{ts,tsx}"
# Also check for TypeScript files in the project
echo "Listing TypeScript files..."
fd "\.(ts|tsx)$"
Length of output: 29944
Suyash878
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
eslint.config.mjs (3)
1-14: LGTM! Consider organizing imports for better maintainability.The imports are comprehensive and include all necessary plugins. Consider organizing them into groups:
- Node.js built-ins
- ESLint core/compat
- ESLint plugins
- Parsers and utilities
+ // Node.js built-ins import path from 'node:path'; import { fileURLToPath } from 'node:url'; + + // ESLint core/compat import js from '@eslint/js'; import { FlatCompat } from '@eslint/eslintrc'; import { fixupPluginRules } from '@eslint/compat'; import globals from 'globals'; + + // ESLint plugins import react from 'eslint-plugin-react'; import typescriptEslint from '@typescript-eslint/eslint-plugin'; import jest from 'eslint-plugin-jest'; import _import from 'eslint-plugin-import'; import tsdoc from 'eslint-plugin-tsdoc'; import prettier from 'eslint-plugin-prettier'; + + // Parsers and utilities import tsParser from '@typescript-eslint/parser';
25-34: Consider adding common build and test directories to ignores.The ignore patterns cover most file types but could be enhanced with common build and test output directories.
ignores: [ '**/*.css', '**/*.scss', '**/*.less', '**/*.json', 'src/components/CheckIn/tagTemplate.ts', '**/package.json', '**/package-lock.json', '**/tsconfig.json', + 'build/**', + 'dist/**', + 'coverage/**', + 'node_modules/**', ],
167-191: Consider adjusting React rule severity levels.Some React rules might be too restrictive or inconsistent in their severity levels:
jsx-equals-spacingis set to "warn" while most other rules are "error"function-component-definitionis effectively disabled (0) but includes configuration- 'react/jsx-equals-spacing': ['warn', 'never'], + 'react/jsx-equals-spacing': ['error', 'never'], - 'react/function-component-definition': [ - 0, - { - namedComponents: 'function-declaration', - }, - ], + 'react/function-component-definition': ['error', { + namedComponents: ['function-declaration', 'arrow-function'], + unnamedComponents: 'arrow-function' + }],
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eslint.config.mjs(1 hunks)
🔇 Additional comments (2)
eslint.config.mjs (2)
36-43: Remove 'prettier' from the extends array to prevent conflicts.
Using both 'plugin:prettier/recommended' and 'prettier' in the extends array is redundant as 'plugin:prettier/recommended' already includes prettier's recommended configuration.
...compat.extends(
'plugin:react/recommended',
'eslint:recommended',
'plugin:jest/recommended',
'plugin:prettier/recommended',
'plugin:@typescript-eslint/recommended',
- 'prettier',
),81-113: Strict TypeScript rules need adjustments before enabling
The enabled TypeScript rules are quite strict and may cause numerous errors in the existing codebase. Consider a gradual approach to enabling these rules.
#!/bin/bash
# Verify impact of strict TypeScript rules
echo "Checking TypeScript files for potential violations..."
# Check for explicit any usage
echo "Files with 'any' type usage:"
rg ":\s*any\b" --type=typescript
# Check for non-null assertions
echo "Files with non-null assertions:"
rg "[!]\." --type=typescript
# Check for type assertions
echo "Files with type assertions:"
rg "as\s+\w+" --type=typescript| '@typescript-eslint/naming-convention': [ | ||
| 'error', | ||
| { | ||
| selector: 'interface', | ||
| format: ['PascalCase'], | ||
| prefix: ['Interface', 'TestInterface'], | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reconsider restrictive interface naming convention.
The current configuration requires interfaces to be prefixed with either 'Interface' or 'TestInterface', which deviates from common TypeScript practices and may make the code more verbose.
{
selector: 'interface',
format: ['PascalCase'],
- prefix: ['Interface', 'TestInterface'],
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| '@typescript-eslint/naming-convention': [ | |
| 'error', | |
| { | |
| selector: 'interface', | |
| format: ['PascalCase'], | |
| prefix: ['Interface', 'TestInterface'], | |
| }, | |
| '@typescript-eslint/naming-convention': [ | |
| 'error', | |
| { | |
| selector: 'interface', | |
| format: ['PascalCase'], | |
| }, |
| parser: tsParser, | ||
| ecmaVersion: 2018, | ||
| sourceType: 'module', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update ECMAVersion to support modern JavaScript features.
The current ECMAVersion (2018) is outdated. Consider updating to a more recent version to support modern JavaScript features.
parser: tsParser,
- ecmaVersion: 2018,
+ ecmaVersion: 2022,
sourceType: 'module',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| parser: tsParser, | |
| ecmaVersion: 2018, | |
| sourceType: 'module', | |
| parser: tsParser, | |
| ecmaVersion: 2022, | |
| sourceType: 'module', |
What kind of change does this PR introduce?
Feature
Issue Number:
Fixes #2645
Did you add tests for your changes?
Not required.
Snapshots/Videos:
None.
If relevant, did you update the documentation?
Not required.
Summary
Successfully migrating from the deprecated
.eslintrc.jsonto the newly supportedeslint.config.jsconfiguration.Does this PR introduce a breaking change?
No.
Other information
None.
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Bug Fixes
Chores