-
Notifications
You must be signed in to change notification settings - Fork 68
feature: upgrade typescript-eslint and node packages to latest versions #1504
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
base: main
Are you sure you want to change the base?
Conversation
- Upgrade @typescript-eslint/eslint-plugin from ^6.2.1 to ^8.14.0 - Upgrade @typescript-eslint/parser from ^6.2.1 to ^8.14.0 - Upgrade eslint from ^8.46.0 to ^9.14.0 - Migrate from legacy .eslintrc.js to flat config eslint.config.js - Update @types/node from ^20.4.6 to ^22.9.0 - Update typescript from ^5.1.6 to ^5.6.3 - Update prettier and related plugins to latest versions - Update Node.js engine requirement from >=18 to >=20 (LTS) - Add @eslint/js dependency for flat config support - Remove outdated eslint plugins no longer needed - Create migration guide documentation - Backup original .eslintrc.js configuration Resolves: credebl#1478 Signed-off-by: DeepDN <nemadedeepak1111@gmail.com>
WalkthroughThis pull request migrates ESLint configuration from v8 (legacy format) to v9 (flat config), updates TypeScript-ESLint and related tooling to major versions, increases the Node.js engine requirement from >=18 to >=20, and includes migration documentation and configuration backups. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.eslintrc.js.backup(1 hunks)ESLINT_MIGRATION.md(1 hunks)eslint.config.js(1 hunks)package.json(2 hunks)
🔇 Additional comments (9)
ESLINT_MIGRATION.md (2)
1-62: Comprehensive migration documentation.The migration guide is well-structured and provides clear instructions for developers. The version updates, configuration changes, and verification steps are all documented appropriately.
25-27: Plugins successfully removed - documentation is accurate.Verification confirms all three plugins have been removed from package.json, old .eslintrc files are gone, and the new flat config structure (eslint.config.js) is in place. The migration documentation correctly describes this state, with functionality covered by the new setup.
.eslintrc.js.backup (1)
1-104: Backup file preserved correctly.The legacy ESLint configuration has been properly backed up for rollback purposes. This will allow reverting to the previous configuration if needed.
package.json (2)
128-128: New dependency for ESLint flat config support.The
@eslint/jspackage is correctly added as a required dependency for ESLint v9's flat configuration format.
135-155: Update pnpm-lock.yaml before verifying compatibility of major version upgrades.The pnpm-lock.yaml is out of sync with package.json. The lockfile contains old versions (e.g., @types/node ^20.4.6, @typescript-eslint/eslint-plugin ^6.2.1) while package.json specifies upgraded versions (^22.9.0, ^8.14.0, etc.). Dependencies could not be installed due to frozen-lockfile mode, preventing verification of build, lint, and test compatibility.
Regenerate the lockfile with:
pnpm install --no-frozen-lockfileThen verify the build and tests pass:
pnpm install pnpm run build pnpm run lint pnpm run testeslint.config.js (4)
1-5: Imports configured correctly for flat config.The imports properly use the new ESLint flat config format with the required packages.
6-8: Base configuration properly extends recommended ruleset.The configuration correctly extends
js.configs.recommendedas the foundation for the ESLint setup.
25-107: Rules preserved from legacy configuration.The comprehensive ruleset has been successfully migrated from the legacy configuration, maintaining consistency in linting standards.
109-111: Ignore patterns configured correctly.The ignore patterns properly exclude the legacy config file, build outputs, and node_modules from linting.
| globals: { | ||
| node: true, | ||
| jest: true, | ||
| }, |
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.
Critical: Incorrect globals syntax for flat config.
The globals are defined incorrectly. In ESLint flat config, globals should use actual global definitions, not boolean values. This won't properly define Node.js and Jest global variables, potentially causing false positives for undefined variables.
Apply this diff to fix the globals definition:
+import globals from 'globals';
+
export default [
js.configs.recommended,
{
files: ['**/*.ts', '**/*.tsx'],
languageOptions: {
parser: tsparser,
parserOptions: {
project: './tsconfig.json',
sourceType: 'module',
},
globals: {
- node: true,
- jest: true,
+ ...globals.node,
+ ...globals.jest,
},
},You'll also need to add the globals package to devDependencies:
"devDependencies": {
"@eslint/js": "^9.14.0",
+ "globals": "^15.12.0",Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In eslint.config.js around lines 16-19, the globals are incorrectly set to
booleans (node: true, jest: true); replace them with actual global definitions
from the external "globals" package by importing/require-ing it and spreading
the corresponding entries (e.g., ...require('globals').node,
...require('globals').jest) into the globals object, and add "globals" to
devDependencies in package.json.
| 'no-lone-blocks': 'error', | ||
| 'no-loop-func': 'error', | ||
| 'no-param-reassign': 'error', | ||
| 'no-return-await': 'error', |
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.
Replace deprecated no-return-await rule.
The no-return-await rule was deprecated in ESLint v8.53.0. For TypeScript projects, use @typescript-eslint/return-await instead, which provides better type-aware handling.
Apply this diff:
- 'no-return-await': 'error',
+ '@typescript-eslint/return-await': 'error',📝 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.
| 'no-return-await': 'error', | |
| '@typescript-eslint/return-await': 'error', |
🤖 Prompt for AI Agents
In eslint.config.js around line 53, replace the deprecated ESLint rule
'no-return-await' with the TypeScript-aware rule
'@typescript-eslint/return-await'; remove the 'no-return-await' entry and add
'@typescript-eslint/return-await' set to ['error', 'never'] (or another
preferred option like 'in-try-catch') so the project uses the supported,
type-aware rule.
| "packageManager": "pnpm@9.15.3+sha512.1f79bc245a66eb0b07c5d4d83131240774642caaa86ef7d0434ab47c0d16f66b04e21e0c086eb61e62c77efc4d7f7ec071afad3796af64892fae66509173893a", | ||
| "engines": { | ||
| "node": ">=18" | ||
| "node": ">=20" |
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.
🧩 Analysis chain
Node.js version requirement increased to v20.
The minimum Node.js version has been bumped from >=18 to >=20. Verify that:
- CI/CD pipelines are configured for Node.js 20
- Development environments meet this requirement
- Deployment environments support Node.js 20
Check the README, CI configuration files, and Dockerfiles to ensure they specify Node.js 20:
🏁 Script executed:
#!/bin/bash
# Search for Node.js version specifications in configuration files
echo "Checking for Node.js version references..."
fd -e yml -e yaml -e json -e md -e dockerfile --exec rg -n "node.*18|18\.x|lts/hydrogen" {}
echo "Checking Docker base images..."
fd -g "*Dockerfile*" --exec rg -n "FROM.*node:" {}Length of output: 9512
Update all Dockerfiles to Node 20 base image to match package.json requirement.
The root package.json now requires Node >=20, but all Dockerfiles still use node:18-alpine base images. This creates a breaking mismatch—Docker containers will fail to satisfy the engine requirement.
Update all 18+ Dockerfiles in ./Dockerfiles/ and root directory from FROM node:18-alpine to FROM node:20-alpine.
🤖 Prompt for AI Agents
In package.json around line 205 the engines field requires "node": ">=20";
update all Dockerfiles that still use Node 18 to match this requirement: in
every Dockerfile in the project root and in ./Dockerfiles/ replace the base
image line (e.g. FROM node:18-alpine or any node:18.* tag) with FROM
node:20-alpine, and rebuild any multi-stage FROM entries or CI scripts that
reference node:18 to node:20 to ensure containers satisfy the package.json
engine constraint.



Resolves: #1478
Summary by CodeRabbit