Migrate eslint to flat config and parser from babel to typescript-eslint #805
Conversation
WalkthroughThe pull request introduces a significant update to the project's ESLint configuration by transitioning from the legacy Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
package.json (1)
Keep Babel presets, remove only eslint parser
The Babel presets are still actively used in your build and test setup:
@babel/preset-reactand@babel/preset-envare required by your.babelrc.jsonconfiguration- Your Jest setup uses
babel-jestfor transforming JavaScript/JSX filesOnly
@babel/eslint-parsercan be safely removed as it's being replaced by typescript-eslint.🔗 Analysis chain
Line range hint
77-79: Remove unnecessary Babel dependencies.Since we're migrating to typescript-eslint, the following Babel-related dependencies can be removed unless they're needed for other purposes:
- "@babel/eslint-parser": "7.23.10", - "@babel/preset-env": "7.24.0", - "@babel/preset-react": "7.22.15",Please verify if these are still needed for other build processes before removing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for babel usage in the project echo "Checking for Babel usage..." rg -l "babel|@babel" --type js --type json --type tsLength of output: 202
Script:
#!/bin/bash # Check Babel configuration and Jest setup echo "=== .babelrc.json content ===" cat .babelrc.json echo -e "\n=== jest.config.ts content ===" cat jest.config.tsLength of output: 1296
🧹 Nitpick comments (1)
eslint.config.js (1)
8-16: Consider adding strict TypeScript configurations.While the current setup is good, consider adding
tseslint.configs.strictfor stricter type checking. This would help catch more TypeScript-specific issues early in development.export default tseslint.config( eslint.configs.recommended, tseslint.configs.recommended, + tseslint.configs.strict, jsxA11y.flatConfigs.recommended,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.eslintrc.cjs(0 hunks)eslint.config.js(1 hunks)package.json(5 hunks)
💤 Files with no reviewable changes (1)
- .eslintrc.cjs
🔇 Additional comments (3)
eslint.config.js (1)
1-7: LGTM! Imports are well-organized and complete.All necessary ESLint plugins and parsers are properly imported, including the typescript-eslint parser as required for the migration.
package.json (2)
62-62: Verify TypeScript file handling in lint script.The simplified lint command relies on ESLint's default file resolution. Verify that TypeScript files are being properly linted.
✅ Verification successful
TypeScript files are properly handled in the lint script
The simplified lint command is safe as the ESLint flat configuration explicitly includes TypeScript files (
**/*.{js,jsx,ts,tsx,cjs}), uses the TypeScript parser, and has TypeScript-specific rules configured.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if TypeScript files are being picked up by the lint command echo "Testing lint command on TypeScript files..." touch test.ts eslint test.ts --print-config . rm test.tsLength of output: 256
Script:
#!/bin/bash # Find ESLint configuration files echo "Searching for ESLint config files..." fd -t f -e js -e json -e yaml -e yml -e cjs "eslint" --exec cat {}Length of output: 1529
81-81: Verify version compatibility between ESLint and TypeScript-related packages.The typescript-eslint version (^8.20.0) seems unusually high compared to the current stable releases. Please verify the version compatibility.
Also applies to: 101-101, 140-141
| languageOptions: { | ||
| globals: { | ||
| ...globals.browser, | ||
| ...globals.node, | ||
| ...globals.jest, | ||
| }, | ||
| parser: typescriptParser, | ||
| parserOptions: { | ||
| ecmaVersion: 'latest', | ||
| sourceType: 'module', | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add React-specific parser options.
The TypeScript parser configuration should include React-specific options for proper JSX handling.
parserOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
+ ecmaFeatures: {
+ jsx: true
+ }
}📝 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.
| languageOptions: { | |
| globals: { | |
| ...globals.browser, | |
| ...globals.node, | |
| ...globals.jest, | |
| }, | |
| parser: typescriptParser, | |
| parserOptions: { | |
| ecmaVersion: 'latest', | |
| sourceType: 'module', | |
| } | |
| }, | |
| languageOptions: { | |
| globals: { | |
| ...globals.browser, | |
| ...globals.node, | |
| ...globals.jest, | |
| }, | |
| parser: typescriptParser, | |
| parserOptions: { | |
| ecmaVersion: 'latest', | |
| sourceType: 'module', | |
| ecmaFeatures: { | |
| jsx: true | |
| } | |
| } | |
| }, |
| rules: { | ||
| '@typescript-eslint/no-unused-vars': 'warn', | ||
| '@typescript-eslint/no-explicit-any': 'warn', | ||
| '@typescript-eslint/no-non-null-asserted-optional-chain': 'warn', | ||
| '@typescript-eslint/ban-ts-comment': 'warn', | ||
| '@typescript-eslint/no-require-imports': 'warn', | ||
|
|
||
| 'no-undef': 'warn', | ||
| 'prefer-const': 'warn', | ||
|
|
||
| 'jsx-a11y/alt-text': 'warn', | ||
| 'jsx-a11y/click-events-have-key-events': 'warn', | ||
| 'jsx-a11y/no-autofocus': 'warn', | ||
| 'jsx-a11y/no-static-element-interactions': 'warn', | ||
| 'jsx-a11y/role-has-required-aria-props': 'warn', | ||
| 'jsx-a11y/role-supports-aria-props': 'warn', | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential TypeScript rules for better type safety.
Consider adding these important TypeScript rules:
rules: {
'@typescript-eslint/no-unused-vars': 'warn',
'@typescript-eslint/no-explicit-any': 'warn',
'@typescript-eslint/no-non-null-asserted-optional-chain': 'warn',
'@typescript-eslint/ban-ts-comment': 'warn',
'@typescript-eslint/no-require-imports': 'warn',
+ '@typescript-eslint/explicit-function-return-type': 'warn',
+ '@typescript-eslint/no-floating-promises': 'warn',
+ '@typescript-eslint/strict-boolean-expressions': 'warn',
'no-undef': 'warn',📝 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.
| rules: { | |
| '@typescript-eslint/no-unused-vars': 'warn', | |
| '@typescript-eslint/no-explicit-any': 'warn', | |
| '@typescript-eslint/no-non-null-asserted-optional-chain': 'warn', | |
| '@typescript-eslint/ban-ts-comment': 'warn', | |
| '@typescript-eslint/no-require-imports': 'warn', | |
| 'no-undef': 'warn', | |
| 'prefer-const': 'warn', | |
| 'jsx-a11y/alt-text': 'warn', | |
| 'jsx-a11y/click-events-have-key-events': 'warn', | |
| 'jsx-a11y/no-autofocus': 'warn', | |
| 'jsx-a11y/no-static-element-interactions': 'warn', | |
| 'jsx-a11y/role-has-required-aria-props': 'warn', | |
| 'jsx-a11y/role-supports-aria-props': 'warn', | |
| } | |
| rules: { | |
| '@typescript-eslint/no-unused-vars': 'warn', | |
| '@typescript-eslint/no-explicit-any': 'warn', | |
| '@typescript-eslint/no-non-null-asserted-optional-chain': 'warn', | |
| '@typescript-eslint/ban-ts-comment': 'warn', | |
| '@typescript-eslint/no-require-imports': 'warn', | |
| '@typescript-eslint/explicit-function-return-type': 'warn', | |
| '@typescript-eslint/no-floating-promises': 'warn', | |
| '@typescript-eslint/strict-boolean-expressions': 'warn', | |
| 'no-undef': 'warn', | |
| 'prefer-const': 'warn', | |
| 'jsx-a11y/alt-text': 'warn', | |
| 'jsx-a11y/click-events-have-key-events': 'warn', | |
| 'jsx-a11y/no-autofocus': 'warn', | |
| 'jsx-a11y/no-static-element-interactions': 'warn', | |
| 'jsx-a11y/role-has-required-aria-props': 'warn', | |
| 'jsx-a11y/role-supports-aria-props': 'warn', | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rollup.config.cjs (1)
Line range hint
31-33: Consider streamlining the TypeScript and Babel setup.Since you're migrating to typescript-eslint, you might want to consider simplifying your build chain. Currently, you're using both TypeScript and Babel with React preset, which might be redundant. TypeScript can handle JSX transformation directly.
Consider:
- Using TypeScript's built-in JSX handling by configuring
jsxintsconfig.json- Removing the Babel React preset if TypeScript is handling JSX
This could simplify your build process and potentially improve build times.
Also applies to: 35-38
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
eslint.config.js(1 hunks)package.json(5 hunks)rollup.config.cjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- eslint.config.js
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
rollup.config.cjs (1)
57-57: Good addition of 'react/jsx-runtime' to externals!This change correctly externalizes the React JSX runtime, which is essential for modern React builds using the new JSX transform. This will help prevent duplicate React runtime code in the bundle.
Let's verify the impact on the bundle output:
✅ Verification successful
Externalization of 'react/jsx-runtime' is correct and future-proof
While there are no direct imports of 'react/jsx-runtime' in the source code, this externalization is beneficial as:
- It aligns with modern React build patterns
- Supports both classic and modern JSX transforms as indicated by the TypeScript configurations
- Prevents potential duplicate React runtime code in the bundle
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if any existing components are importing jsx-runtime directly # This helps verify if the externalization is necessary # Search for jsx-runtime imports in component files rg -t tsx -t jsx "from\s+['\"]react/jsx-runtime['\"]" src/components/ui/Length of output: 106
Script:
#!/bin/bash # Check for jsx-runtime imports in all JS/TS files fd -e js -e jsx -e ts -e tsx . src/ -x rg -l "from\s+['\"]react/jsx-runtime['\"]" {} \; # Check for JSX transform configuration in various config files echo "=== TypeScript Config ===" fd tsconfig.json -x cat {} \; echo -e "\n=== Babel Config ===" fd -e js -e json -e cjs -name "*babel*" -x cat {} \;Length of output: 1921
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
eslint.config.js (2)
19-30: 🛠️ Refactor suggestionAdd React-specific parser options.
The TypeScript parser configuration should include React-specific options for proper JSX handling.
parserOptions: { ecmaVersion: 'latest', sourceType: 'module', + ecmaFeatures: { + jsx: true + } }
31-48: 🛠️ Refactor suggestionAdd essential TypeScript rules for better type safety.
Consider adding these important TypeScript rules:
rules: { '@typescript-eslint/no-unused-vars': 'warn', '@typescript-eslint/no-explicit-any': 'warn', '@typescript-eslint/no-non-null-asserted-optional-chain': 'warn', '@typescript-eslint/ban-ts-comment': 'warn', '@typescript-eslint/no-require-imports': 'warn', + '@typescript-eslint/explicit-function-return-type': 'warn', + '@typescript-eslint/no-floating-promises': 'warn', + '@typescript-eslint/strict-boolean-expressions': 'warn', 'no-undef': 'warn',
🧹 Nitpick comments (2)
eslint.config.js (2)
3-8: Consider organizing imports for better maintainability.Group related imports together for better readability:
import eslint from '@eslint/js'; import globals from 'globals'; + import tseslint from 'typescript-eslint'; import typescriptParser from '@typescript-eslint/parser'; + import react from 'eslint-plugin-react'; import jsxA11y from 'eslint-plugin-jsx-a11y';
31-48: Consider using 'error' for critical rules.All rules are currently set to 'warn'. Consider upgrading critical rules to 'error' level for better code quality enforcement:
rules: { - '@typescript-eslint/no-explicit-any': 'warn', + '@typescript-eslint/no-explicit-any': 'error', - '@typescript-eslint/no-non-null-asserted-optional-chain': 'warn', + '@typescript-eslint/no-non-null-asserted-optional-chain': 'error', - 'no-undef': 'warn', + 'no-undef': 'error',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eslint.config.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
| export default tseslint.config( | ||
| eslint.configs.recommended, | ||
| tseslint.configs.recommended, | ||
| jsxA11y.flatConfigs.recommended, | ||
| { | ||
| files: [ '**/*.{js,jsx,ts,tsx,cjs}' ], | ||
| plugins: { | ||
| react, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add React recommended configuration.
The React plugin is imported but its recommended configuration is not enabled.
export default tseslint.config(
eslint.configs.recommended,
tseslint.configs.recommended,
jsxA11y.flatConfigs.recommended,
+ react.configs.recommended,
{
files: [ '**/*.{js,jsx,ts,tsx,cjs}' ],
plugins: {
react,
},📝 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.
| export default tseslint.config( | |
| eslint.configs.recommended, | |
| tseslint.configs.recommended, | |
| jsxA11y.flatConfigs.recommended, | |
| { | |
| files: [ '**/*.{js,jsx,ts,tsx,cjs}' ], | |
| plugins: { | |
| react, | |
| }, | |
| export default tseslint.config( | |
| eslint.configs.recommended, | |
| tseslint.configs.recommended, | |
| jsxA11y.flatConfigs.recommended, | |
| react.configs.recommended, | |
| { | |
| files: [ '**/*.{js,jsx,ts,tsx,cjs}' ], | |
| plugins: { | |
| react, | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
eslint.config.js (3)
10-14: 🛠️ Refactor suggestionAdd React recommended configuration.
The React plugin is imported but its recommended configuration is not enabled.
export default tseslint.config( eslint.configs.recommended, tseslint.configs.recommended, jsxA11y.flatConfigs.recommended, + react.configs.recommended, {
20-31: 🛠️ Refactor suggestionAdd React-specific parser options.
The TypeScript parser configuration should include React-specific options for proper JSX handling.
parserOptions: { ecmaVersion: 'latest', sourceType: 'module', + ecmaFeatures: { + jsx: true + } }
32-38: 🛠️ Refactor suggestionAdd essential TypeScript rules for better type safety.
Consider adding these important TypeScript rules:
rules: { '@typescript-eslint/no-unused-vars': 'warn', '@typescript-eslint/no-explicit-any': 'warn', '@typescript-eslint/no-non-null-asserted-optional-chain': 'warn', '@typescript-eslint/ban-ts-comment': 'warn', '@typescript-eslint/no-require-imports': 'warn', + '@typescript-eslint/explicit-function-return-type': 'warn', + '@typescript-eslint/no-floating-promises': 'warn', + '@typescript-eslint/strict-boolean-expressions': 'warn',
🧹 Nitpick comments (1)
eslint.config.js (1)
19-19: Expand the ignores list for better coverage.Consider adding more common paths to the ignores list:
- ignores: ['assets/', 'dist/'], + ignores: [ + 'assets/', + 'dist/', + 'node_modules/', + 'coverage/', + 'build/', + '**/*.min.js', + '**/*.d.ts' + ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eslint.config.js(1 hunks)
🔇 Additional comments (2)
eslint.config.js (2)
1-9: LGTM! Imports and setup are well-structured.The file includes all necessary imports and the
@ts-checkdirective for TypeScript checking.
15-15: Verify TypeScript files coverage.Let's ensure all TypeScript files in the project will be properly linted with this configuration.
✅ Verification successful
TypeScript files coverage is complete
The glob pattern
**/*.{js,jsx,cjs,ts,tsx}will properly lint all TypeScript files in the project, including components, configurations, and utilities in both/srcand/docsdirectories.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any TypeScript files that might be missed by the current configuration # Find all TypeScript files echo "TypeScript files in the project:" fd -e ts -e tsx # Check for any TypeScript files in directories that might be excluded echo -e "\nTypeScript files in potentially excluded directories:" fd -e ts -e tsx -p "test/" "src/" "lib/" "packages/"Length of output: 9195
kotAPI
added
the
automerge
3 participants
Start to improve linting by migrating to the new flat config for eslint and switching from babel to typescript-eslint as the parser
Epic:
Relevant issues:
Summary by CodeRabbit
Chores
Refactor
.eslintrc.cjswith neweslint.config.jsconfiguration