-
Notifications
You must be signed in to change notification settings - Fork 64
Migrate and test project to latest versions #745
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: master
Are you sure you want to change the base?
Migrate and test project to latest versions #745
Conversation
- Migrated from deprecated TSLint to ESLint v8 with TypeScript support - Updated Node.js requirement from >=8.0.0 to >=18.0.0 (latest LTS) - Updated TypeScript to v5.9.2 with ES2022 target and strict mode enabled - Updated all dependencies to latest compatible versions: - argon2: ^0.31.1 → ^0.44.0 - body-parser: ^1.20.0 → ^2.2.0 - dotenv: ^16.0.3 → ^17.2.1 - express: ^4.18.1 → ^5.1.0 - helmet: ^7.0.0 → ^8.1.0 - joi: ^17.6.0 → ^18.0.1 - mongodb: ^6.2.0 → ^6.19.0 - reflect-metadata: ^0.1.13 → ^0.2.2 - typeorm: ^0.3.10 → ^0.3.26 - Fixed import syntax from namespace imports to modern ES module imports - Added Jest configuration file for better testing setup - Fixed TypeScript strict mode compatibility issues - Updated exception handling and error types - Added proper type annotations for middleware functions - Fixed linting issues and code formatting All tests pass and application builds successfully.
|
Cursor Agent can help with this pull request. Just |
WalkthroughIntroduces development environment variables, migrates linting from TSLint to ESLint, adds Jest config, modernizes TypeScript/Node targets, and upgrades dependencies. Refactors imports/typing in Express code, adjusts configuration parsing and defaults, updates Exception class structure, tightens model typings, and changes a service method’s return type. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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.
Pull Request Overview
This PR modernizes the project by migrating from TSLint to ESLint, upgrading Node.js requirements, updating TypeScript to v5.x with strict mode, and modernizing import syntax for better type safety and compatibility.
- Replaces TSLint with ESLint configuration and updates linting scripts
- Modernizes import statements from CommonJS-style to ES modules
- Updates TypeScript strict mode with definite assignment assertions and proper type annotations
Reviewed Changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tslint.json | Removes deprecated TSLint configuration file |
| .eslintrc.js | Adds new ESLint configuration with TypeScript support |
| package.json | Updates dependencies to latest versions and changes lint scripts |
| src/services/users.ts | Updates return type to include null for TypeScript strict mode |
| src/services/auth.ts | Modernizes imports and improves error handling |
| src/models/User.ts | Adds definite assignment assertions for strict TypeScript |
| src/main.ts | Updates Express import syntax |
| src/loaders/*.ts | Modernizes imports and improves type annotations |
| src/config/index.ts | Updates dotenv usage and adds fallback values |
| src/api/**/*.ts | Adds proper type annotations for middleware functions |
| src/Exception/index.ts | Refactors class export and improves property access pattern |
| .env | Adds environment configuration file |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
tsconfig.json (1)
1-64: Add useDefineForClassFields: false to avoid TypeORM/TS5 class-field surprises.With TS 5 targeting ES2022, define semantics are enabled by default and can break some decorator-based ORMs. Set this explicitly to be safe.
"compilerOptions": { + "useDefineForClassFields": false,src/api/middlewares/attachCurrentUser.ts (1)
10-28: Type safety, error guard, and logging polish for middleware.
- Avoid any; type the middleware and augment Express.Request.
- Guard when req.auth is missing to prevent runtime crash.
- Prefer non-mutating password/salt removal and use console.error per lint rules.
-const attachCurrentUser = async (req: any, res: any, next: any) => { +const attachCurrentUser = async (req: AugmentedRequest, res: Response, next: NextFunction) => { try { - const userServiceInstance = Container.get(UserService); - const userRecord = await userServiceInstance.findOne(req.auth.id); + if (!req.auth?.id) return res.sendStatus(401); + const userServiceInstance = Container.get(UserService); + const userRecord = await userServiceInstance.findOne(req.auth.id); if (!userRecord) { return res.sendStatus(401); } - const currentUser = userRecord; - Reflect.deleteProperty(currentUser, 'password'); - Reflect.deleteProperty(currentUser, 'salt'); - req.currentUser = currentUser; - req.currentUser.os = req.auth.os; + const { password, salt, ...safeUser } = userRecord as Record<string, unknown>; + req.currentUser = { ...safeUser, os: req.auth?.os }; return next(); } catch (e) { - console.log(' Error attaching user to req'); - console.log(e); + console.error('Error attaching user to req', e); return next(e); } };Add these supporting types/imports (outside the diffed range):
// at top of file import type { Request, Response, NextFunction } from 'express'; // in the same file or a shared types module type AugmentedRequest = Request & { auth?: { id: string; os?: string }; currentUser?: Record<string, unknown> & { os?: string }; };Optionally, formalize this via ambient declaration:
- Create src/types/express/index.d.ts:
import 'express'; declare module 'express-serve-static-core' { interface Request { auth?: { id: string; os?: string }; currentUser?: Record<string, unknown> & { os?: string }; } }src/api/routes/auth/index.ts (3)
27-33: Fix status code chaining: set status before json.
res.json(...).status(201)sends a 200; status set after send is ignored.Apply this diff:
- const { user, token } = await authServiceInstance.SignUp(req.body as User); - return res.json({ user, token }).status(201); + const { user, token } = await authServiceInstance.SignUp(req.body as User); + return res.status(201).json({ user, token });
47-53: Same status chaining bug on login.Set status first to reliably send 200 (or 201 for created).
Apply this diff:
- const { user, token } = await authServiceInstance.SignIn(email, password); - return res.json({ user, token }).status(200); + const { user, token } = await authServiceInstance.SignIn(email, password); + return res.status(200).json({ user, token });
20-26: Reject unknown keys at the boundary.Without
.unknown(false), extra fields (e.g., role) pass through validation and can lead to privilege-escalation bugs if used downstream.Apply this diff:
- celebrate({ - body: Joi.object({ + celebrate({ + body: Joi.object({ name: Joi.string().min(2).max(30).required(), email: Joi.string().trim().email().required(), password: Joi.string().min(8).max(30).required(), - }), + }).unknown(false), }),(Repeat the same change for the login schema.)
Also applies to: 41-46
src/loaders/server.ts (2)
13-16: Celebrate error middleware is mounted before routes; it won’t catch route validation errors.Middleware order matters; mount
errors()after routes and before 404/error handlers.Apply:
- app.use(errors()); - - app.use('/api', routes); + app.use('/api', routes); + app.use(errors()); // celebrate error handler must come after routes
36-38: Serialize Celebrate error details; Map won’t JSON-stringify.
err.detailsis a Map; sending it yields{}. Convert to a plain object/array.Apply:
- if (isCelebrateError(err)) { - return res.status(422).send({ message: err.message, details: err.details }).end(); - } + if (isCelebrateError(err)) { + const details = Array.from(err.details.entries()).reduce((acc: any, [segment, joiError]: any) => { + acc[segment] = joiError.details?.map((d: any) => d.message) ?? []; + return acc; + }, {}); + return res.status(422).json({ message: err.message, details }); + }
🧹 Nitpick comments (23)
package.json (2)
27-28: Consider dropping body-parser in favor of built-in Express parsers.Express 4.16+ (and 5) expose express.json() and express.urlencoded(); removing body-parser trims one dependency.
I can scan usages and propose a patch if wanted.
15-17: Add an explicit dev runner for TS (nodemon exec).Start script is plain “nodemon”; if there’s no nodemon.json, it won’t run TS. Suggest:
- "start": "nodemon", + "start": "nodemon --exec ts-node src/main.ts",Or switch to tsx:
+ "dev": "tsx watch src/main.ts",.eslintrc.js (1)
1-30: ESLint baseline looks solid; consider a faster, isolated project config.Using parserOptions.project against the full tsconfig can slow linting. Consider a tsconfig.eslint.json (noEmit, includes tests) to speed up and widen coverage. Also, you’re warning on any while using it in middleware—tighten types where feasible.
jest.config.js (1)
23-24: Include cts/mts extensions if you keep Jest 30.Jest 30 adds support for .cts/.mts by default; since you override moduleFileExtensions, include them.
- moduleFileExtensions: ['ts', 'js', 'json'], + moduleFileExtensions: ['ts', 'cts', 'mts', 'js', 'json'],src/models/User.ts (2)
45-46: Drop SQL-only enum metadata for Mongo.
type: 'enum'isn’t meaningful for Mongo. Keepenumdefault in TS, but removetype.- @Column({ type: 'enum', enum: Role, default: Role.User }) + @Column({ enum: Role, default: Role.User }) public role!: Role;
48-54: Redundant @column with date columns.
@CreateDateColumn()/@UpdateDateColumn()don’t need an extra@Column().- @Column() @CreateDateColumn() public createdAt!: Date; - @Column() @UpdateDateColumn() public updatedAt!: Date;src/config/index.ts (3)
6-9: Throwing when .env is missing can break prod.Prefer: warn if missing but continue when env vars are provided; or only throw in local dev.
-const envFound = dotenv.config(); -if (envFound.error) { - // Throw generic error - throw new Error("Couldn't find .env file"); -} +const envFound = dotenv.config(); +if (envFound.error && process.env.NODE_ENV === 'development') { + throw new Error("Couldn't find .env file"); +}
15-15: Use explicit radix for parseInt.Avoid implicit base.
- port: parseInt(process.env.PORT || '3000'), + port: Number.parseInt(process.env.PORT || '3000', 10),
54-58: Normalize numeric envs.
pooltimeandconcurrencylikely need numbers; consider parsing like the other ports.src/api/middlewares/isAuth.ts (1)
22-26: Consider validating issuer/audience if applicable.If your tokens include iss/aud, enforce them here to harden verification.
src/api/routes/auth/index.ts (2)
15-15: Avoidanyfor the router argument. Type it asRouter.You already import
Router; this improves type safety.Apply this diff:
-export default (app: any) => { +export default (app: Router) => {
33-33: Useconsole.errorfor errors or your app logger.Improves signal in logs and integrates with error monitoring.
Apply this diff:
- console.log(' error ', e); + console.error('error', e);Also applies to: 54-54, 68-68
src/loaders/index.ts (2)
1-1: Prefer importing theApplicationtype explicitly.Avoids relying on the namespace off the default import.
Apply this diff:
-import express from 'express'; +import express, { type Application } from 'express'; -export default async (app: express.Application) => { +export default async (app: Application) => {
7-12: Startup logs: consider using a centralized logger.Consistency with the rest of the app and structured logging helps during upgrades.
src/main.ts (1)
9-18: Graceful shutdown: close the server beforeprocess.exit(0).Prevents dropping in-flight requests on SIGTERM.
Example replacement:
const server = app.listen(config.port, () => { console.log(`Server is listening on port ${config.port}`); }); process.on('SIGTERM', () => { console.log('SIGTERM signal received.'); server.close(() => { console.log('HTTP server closed.'); process.exit(0); }); });src/services/users.ts (1)
10-12: Drop unnecessaryawaitin simple returns.Removes microtask overhead and aligns with common style.
Apply this diff:
- public async findAll(): Promise<User[]> { - return await this.userRepository.find(); - } + public async findAll(): Promise<User[]> { + return this.userRepository.find(); + } - public async findOne(options: FindOneOptions<User>): Promise<User | null> { - return await this.userRepository.findOne(options); - } + public async findOne(options: FindOneOptions<User>): Promise<User | null> { + return this.userRepository.findOne(options); + }Also applies to: 14-16
src/services/auth.ts (2)
22-27: Avoid manual salt handling; let argon2 embed salt and stop persisting it.Argon2 returns an encoded hash with the salt. Storing a separate
saltis redundant and increases surface area.Apply:
-import { randomBytes } from 'crypto'; @@ - const salt = randomBytes(32); @@ - const hashedPassword = await argon2.hash(inputUser.password, { salt }); + const hashedPassword = await argon2.hash(inputUser.password); @@ - salt: salt.toString('hex'), password: hashedPassword,Optionally remove later
Reflect.deleteProperty(user, 'salt')sincesaltwon’t exist anymore.
85-93: Consider setting JWT options explicitly (algorithm/issuer/audience).Improves interoperability and validation downstream; keep as-is if not needed.
Example:
- return jwt.sign( + return jwt.sign( { id: user.id, role: user.role, name: user.name, exp: exp.getTime() / 1000, }, - config.jwtSecret, + config.jwtSecret, + { algorithm: 'HS256', issuer: config.jwtIssuer, audience: config.jwtAudience } );.env (1)
16-16: Add trailing newline.Keeps linters and POSIX tools happy.
-AGENDA_CONCURRENCY=10⏎? # ensure file ends with a newline +AGENDA_CONCURRENCY=10 +src/loaders/server.ts (4)
1-1: Prefer built-inexpress.json()over body-parser.Removes an extra dependency and aligns with modern Express.
Apply:
-import bodyParser from 'body-parser'; @@ - app.use(bodyParser.json()); + app.use(express.json());Then remove
body-parserfrom dependencies.Also applies to: 12-12
19-21: Type the 404 handler and avoidanymutation.Use Express types and a narrow error shape.
Apply:
- app.use((req, res, next) => { - const error: any = new Error('Not Found'); - error.status = 404; + app.use((req: express.Request, res: express.Response, next: express.NextFunction) => { + const error = new Error('Not Found') as { status?: number }; + error.status = 404; next(error); });
25-40: Tighten error-handler types and early-returns.Use
unknownand narrow when needed; improves strict-mode signal without changing behavior.Apply:
- app.use((err: any, req: any, res: any, next: any) => { + app.use((err: unknown, req: express.Request, res: express.Response, next: express.NextFunction) => { if ((err as any).name === 'UnauthorizedError') { - return res.status(err.status).send({ message: err.message }).end(); + const e = err as any; + return res.status(e.status || 401).json({ message: e.message }); } // celebrate handled above; fall through return next(err); });
42-49: Final error handler: avoidanyand normalize response shape.Small type cleanup; no behavior change.
Apply:
- app.use((err: any, req: any, res: any, _next: any) => { - res.status(err.status || 500); - res.json({ + app.use((err: unknown, req: express.Request, res: express.Response, _next: express.NextFunction) => { + const e = err as { status?: number; message?: string }; + res.status(e?.status ?? 500).json({ errors: { - message: err.message, + message: e?.message ?? 'Internal Server Error', }, }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.env(1 hunks).eslintrc.js(1 hunks)jest.config.js(1 hunks)package.json(1 hunks)src/Exception/index.ts(2 hunks)src/api/middlewares/attachCurrentUser.ts(1 hunks)src/api/middlewares/isAuth.ts(2 hunks)src/api/routes/auth/index.ts(1 hunks)src/config/index.ts(3 hunks)src/loaders/index.ts(1 hunks)src/loaders/server.ts(3 hunks)src/main.ts(1 hunks)src/models/User.ts(1 hunks)src/services/auth.ts(2 hunks)src/services/users.ts(1 hunks)tsconfig.json(2 hunks)tslint.json(0 hunks)
💤 Files with no reviewable changes (1)
- tslint.json
🧰 Additional context used
🧬 Code graph analysis (5)
src/api/routes/auth/index.ts (3)
src/loaders/index.ts (1)
app(6-12)src/loaders/server.ts (1)
app(8-50)src/api/routes/users/index.ts (1)
app(6-17)
src/main.ts (2)
src/loaders/index.ts (1)
app(6-12)src/loaders/server.ts (1)
app(8-50)
src/loaders/index.ts (1)
src/loaders/server.ts (1)
app(8-50)
src/models/User.ts (1)
src/models/ValueTransformers.ts (1)
lowercase(21-28)
src/loaders/server.ts (1)
src/api/routes/auth/index.ts (1)
app(15-73)
🪛 dotenv-linter (3.3.0)
.env
[warning] 3-3: [UnorderedKey] The JWT_SECRET key should go before the NODE_ENV key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The TYPEORM_PASSWORD key should go before the TYPEORM_PORT key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The TYPEORM_DATABASE key should go before the TYPEORM_HOST key
(UnorderedKey)
[warning] 16-16: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 16-16: [UnorderedKey] The AGENDA_CONCURRENCY key should go before the AGENDA_DB_COLLECTION key
(UnorderedKey)
🪛 ESLint
src/api/middlewares/isAuth.ts
[error] 18-18: Definition for rule '@typescript-eslint/no-null-keyword' was not found.
(@typescript-eslint/no-null-keyword)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (11)
tsconfig.json (2)
4-9: Target/lib bump to ES2022 is appropriate for Node 18+.No action needed.
26-26: Strict mode on is a good move.This aligns with the repo-wide TS tightening.
package.json (1)
22-23: Engine bump to Node >=18 is correct and aligns with dependency baselines.LGTM.
src/models/User.ts (2)
20-20: Strict-mode: definite assignment on entity fields looks good.Keeps the model TS5-strict-friendly without runtime impact.
Also applies to: 23-23, 31-31, 37-37, 43-43, 46-46, 50-50, 54-54
33-44: Confirm select:false behavior with MongoDB driver.Historically,
select: falsehasn’t been honored for Mongo reads. Ensure sensitive fields aren’t exposed; if needed, exclude via class-transformer or a customtoJSON.src/Exception/index.ts (1)
72-74: Safer stack-trace capability check.Good improvement switching to
Object.prototype.hasOwnProperty.call.src/config/index.ts (2)
26-31: Clarifydatabase.urlvs host/port.
urlis set toTYPEORM_HOST, which looks wrong. Use a proper connection URL or dropurlwhen supplying host/port separately.
1-1: esModuleInterop is enabled in tsconfig.json (line 51). Default import for dotenv is valid.src/main.ts (1)
6-6: LGTM on Express default import + init.Matches the repo-wide migration.
src/services/users.ts (1)
14-16: Return type widened toUser | nullis correct for TypeORM ≥0.3; ensure call sites handlenull.Scan consumers and guard against
nullto avoid runtime errors.src/services/auth.ts (1)
1-3: No action needed: ESModule interop flags are enabled
tsconfig.json already sets"esModuleInterop": trueand"allowSyntheticDefaultImports": true.
| NODE_ENV=development | ||
| PORT=3000 | ||
| JWT_SECRET=test-secret-key-for-development | ||
|
|
||
| # Database Configuration | ||
| TYPEORM_CONNECTION=mongodb | ||
| TYPEORM_HOST=localhost | ||
| TYPEORM_PORT=27017 | ||
| TYPEORM_USERNAME= | ||
| TYPEORM_PASSWORD= | ||
| TYPEORM_DATABASE=nodejs_boilerplate | ||
|
|
||
| # Agenda Configuration | ||
| AGENDA_DB_COLLECTION=jobs | ||
| AGENDA_POOL_TIME=200 | ||
| AGENDA_CONCURRENCY=10 No newline at end of 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.
Do not commit .env with secrets; switch to .env.example and gitignore .env.
Even dev secrets get reused; committing them is a recurring incident vector.
Suggested actions:
- Remove
.envfrom VCS, rotate any leaked secrets. - Add
.envto.gitignore, keep a.env.example(no secrets).
Example .gitignore update:
+/.env
+/.env.*.local
+!.env.exampleExample .env.example:
NODE_ENV=development
PORT=3000
JWT_SECRET=change-me
TYPEORM_CONNECTION=mongodb
TYPEORM_HOST=localhost
TYPEORM_PORT=27017
TYPEORM_USERNAME=
TYPEORM_PASSWORD=
TYPEORM_DATABASE=nodejs_boilerplate
AGENDA_DB_COLLECTION=jobs
AGENDA_POOL_TIME=200
AGENDA_CONCURRENCY=10
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 3-3: [UnorderedKey] The JWT_SECRET key should go before the NODE_ENV key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The TYPEORM_PASSWORD key should go before the TYPEORM_PORT key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The TYPEORM_DATABASE key should go before the TYPEORM_HOST key
(UnorderedKey)
[warning] 16-16: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 16-16: [UnorderedKey] The AGENDA_CONCURRENCY key should go before the AGENDA_DB_COLLECTION key
(UnorderedKey)
🤖 Prompt for AI Agents
.env lines 1-16: The committed .env contains secrets — remove the file from
version control, rotate any leaked secrets (e.g., JWT_SECRET), and stop tracking
it: add .env to .gitignore and run git rm --cached .env (then commit). Create
and commit a .env.example (no real secrets, use placeholders like
JWT_SECRET=change-me) containing the same keys shown in these lines so
developers have a template, and update repository docs/README to instruct
contributors to create their own local .env from .env.example.
| module.exports = { | ||
| preset: 'ts-jest', | ||
| testEnvironment: 'node', | ||
| roots: ['<rootDir>/src', '<rootDir>/tests'], | ||
| testMatch: [ | ||
| '**/__tests__/**/*.ts', | ||
| '**/?(*.)+(spec|test).ts' | ||
| ], | ||
| transform: { | ||
| '^.+\\.ts$': 'ts-jest', | ||
| }, | ||
| collectCoverageFrom: [ | ||
| 'src/**/*.ts', | ||
| '!src/**/*.d.ts', | ||
| '!src/main.ts', | ||
| ], | ||
| coverageDirectory: 'coverage', | ||
| coverageReporters: [ | ||
| 'text', | ||
| 'lcov', | ||
| 'html' | ||
| ], | ||
| moduleFileExtensions: ['ts', 'js', 'json'], | ||
| testTimeout: 30000, | ||
| }; No newline at end of 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.
Config uses ts-jest with Jest 30—this combo is risky today.
Given the versioning policy, ts-jest 29 targets Jest 29. Either:
- Downgrade Jest to 29 (see package.json suggestion), or
- Replace ts-jest with babel-jest (or swc) and add @babel/preset-typescript.
This prevents test runner failures. (npmjs.com)
🤖 Prompt for AI Agents
In jest.config.js around lines 1 to 25, the project currently uses ts-jest while
Jest 30 is installed which can cause incompatibilities; either pin Jest to v29
or migrate off ts-jest: to fix, choose one approach — (A) Downgrade Jest to v29
in package.json and reinstall so ts-jest v29 is matched (no config change
needed), or (B) Replace ts-jest by installing babel-jest and
@babel/preset-typescript, update devDependencies, remove the ts-jest transform
entry and instead configure transform to use babel-jest (and add a
babel.config.js with @babel/preset-typescript and @babel/preset-env), then
reinstall and run tests to confirm.
| "@types/jest": "^30.0.0", | ||
| "@types/morgan": "^1.9.10", | ||
| "@types/node": "^24.3.0", | ||
| "@typescript-eslint/eslint-plugin": "^6.21.0", | ||
| "@typescript-eslint/parser": "^6.21.0", | ||
| "chai": "^6.0.1", | ||
| "eslint": "^8.57.0", | ||
| "eslint-config-prettier": "^10.1.8", | ||
| "eslint-plugin-prettier": "^5.5.4", | ||
| "jest": "^30.1.2", | ||
| "nodemon": "^3.1.10", | ||
| "prettier": "^3.6.2", | ||
| "ts-jest": "^29.4.1", | ||
| "ts-node": "^10.9.2", | ||
| "typescript": "^5.9.2" | ||
| } |
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.
Version mismatch: Jest 30 with ts-jest 29 is not supported.
ts-jest’s major tracks Jest’s major; 29.x is for Jest 29 and is not guaranteed for Jest 30. Either downgrade Jest to 29 or replace ts-jest (e.g., babel-jest).
- Evidence: Jest 30 release/migration (Node 18+, TS ≥5.4). (github.com, jestjs.io)
- ts-jest docs: “major number follows the one of Jest.” Latest is still 29.x. (npmjs.com)
Minimal fix (stay with ts-jest):
- "@types/jest": "^30.0.0",
- "jest": "^30.1.2",
+ "@types/jest": "^29.5.12",
+ "jest": "^29.7.0",
"ts-jest": "^29.4.1",Alternatively, keep Jest 30 and switch transforms to babel-jest; I can draft that if you prefer.
📝 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.
| "@types/jest": "^30.0.0", | |
| "@types/morgan": "^1.9.10", | |
| "@types/node": "^24.3.0", | |
| "@typescript-eslint/eslint-plugin": "^6.21.0", | |
| "@typescript-eslint/parser": "^6.21.0", | |
| "chai": "^6.0.1", | |
| "eslint": "^8.57.0", | |
| "eslint-config-prettier": "^10.1.8", | |
| "eslint-plugin-prettier": "^5.5.4", | |
| "jest": "^30.1.2", | |
| "nodemon": "^3.1.10", | |
| "prettier": "^3.6.2", | |
| "ts-jest": "^29.4.1", | |
| "ts-node": "^10.9.2", | |
| "typescript": "^5.9.2" | |
| } | |
| "@types/jest": "^29.5.12", | |
| "@types/morgan": "^1.9.10", | |
| "@types/node": "^24.3.0", | |
| "@typescript-eslint/eslint-plugin": "^6.21.0", | |
| "@typescript-eslint/parser": "^6.21.0", | |
| "chai": "^6.0.1", | |
| "eslint": "^8.57.0", | |
| "eslint-config-prettier": "^10.1.8", | |
| "eslint-plugin-prettier": "^5.5.4", | |
| "jest": "^29.7.0", | |
| "nodemon": "^3.1.10", | |
| "prettier": "^3.6.2", | |
| "ts-jest": "^29.4.1", | |
| "ts-node": "^10.9.2", | |
| "typescript": "^5.9.2" |
🤖 Prompt for AI Agents
In package.json around lines 45-60, the project declares jest@30 while keeping
ts-jest@29 which is incompatible; fix by either downgrading jest to a 29.x
release that matches ts-jest (replace "jest": "^30.1.2" with a 29.x semver) or
remove ts-jest and switch to a Babel-based transformer (add babel-jest and the
appropriate @babel/preset-typescript and update the Jest transform config) —
pick the minimal option (downgrade Jest to 29.x) unless you want to convert the
test pipeline to babel-jest.
| const getTokenFromHeader = (req: any) => { | ||
| const { authorization } = req.headers; | ||
| if ( | ||
| (authorization && authorization.split(' ')[0] === 'Token') || | ||
| (authorization && authorization.split(' ')[0] === 'Bearer') | ||
| ) { | ||
| return authorization.split(' ')[1]; | ||
| } | ||
| // tslint:disable-next-line: no-null-keyword | ||
| // eslint-disable-next-line @typescript-eslint/no-null-keyword | ||
| return null; |
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.
🛠️ Refactor suggestion
Token extractor: return undefined, drop broken ESLint disable, and handle scheme case-insensitively.
Prevents the reported rule error and makes parsing more robust.
Apply this diff:
-const getTokenFromHeader = (req: any) => {
- const { authorization } = req.headers;
- if (
- (authorization && authorization.split(' ')[0] === 'Token') ||
- (authorization && authorization.split(' ')[0] === 'Bearer')
- ) {
- return authorization.split(' ')[1];
- }
- // eslint-disable-next-line @typescript-eslint/no-null-keyword
- return null;
-};
+const getTokenFromHeader = (req: any) => {
+ const { authorization } = req.headers || {};
+ if (!authorization) return undefined;
+ const [scheme, token] = authorization.split(' ');
+ if (!token) return undefined;
+ const s = (scheme || '').toLowerCase();
+ if (s === 'bearer' || s === 'token') return token;
+ return undefined;
+};📝 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.
| const getTokenFromHeader = (req: any) => { | |
| const { authorization } = req.headers; | |
| if ( | |
| (authorization && authorization.split(' ')[0] === 'Token') || | |
| (authorization && authorization.split(' ')[0] === 'Bearer') | |
| ) { | |
| return authorization.split(' ')[1]; | |
| } | |
| // tslint:disable-next-line: no-null-keyword | |
| // eslint-disable-next-line @typescript-eslint/no-null-keyword | |
| return null; | |
| const getTokenFromHeader = (req: any) => { | |
| const { authorization } = req.headers || {}; | |
| if (!authorization) return undefined; | |
| const [scheme, token] = authorization.split(' '); | |
| if (!token) return undefined; | |
| const s = (scheme || '').toLowerCase(); | |
| if (s === 'bearer' || s === 'token') return token; | |
| return undefined; | |
| }; |
🧰 Tools
🪛 ESLint
[error] 18-18: Definition for rule '@typescript-eslint/no-null-keyword' was not found.
(@typescript-eslint/no-null-keyword)
🤖 Prompt for AI Agents
In src/api/middlewares/isAuth.ts around lines 10 to 19, the token extractor
currently returns null, disables ESLint, and compares the auth scheme
case-sensitively; update it to return undefined (not null) to satisfy the
reported rule, remove the eslint-disable comment, and parse the Authorization
header robustly by checking header existence, splitting it once, lowercasing the
scheme for case-insensitive comparison against 'token' or 'bearer', and
returning the token part (or undefined if missing).
| getToken: getTokenFromHeader, // How to extract the JWT from the request | ||
| }); |
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.
Do not ship with a default JWT secret. Fail fast if JWT_SECRET is missing.
Hardcoding a fallback like 'default-secret-key' is a critical security risk; tokens become trivially forgeable. Throw at startup unless explicitly running tests.
Apply this diff:
import { expressjwt as jwt } from 'express-jwt';
import config from '../../config';
+// Ensure JWT secret is configured (fail fast outside tests)
+if (!config.jwtSecret && process.env.NODE_ENV !== 'test') {
+ throw new Error('JWT secret is required. Set JWT_SECRET in the environment.');
+}
+
const isAuth = jwt({
algorithms: ['HS256'], // This is the default algorithm used by jsonwebtoken
- secret: config.jwtSecret || 'default-secret-key', // The _secret_ to sign the JWTs
+ secret: config.jwtSecret as string, // The _secret_ to sign the JWTs
getToken: getTokenFromHeader, // How to extract the JWT from the request
});📝 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.
| secret: config.jwtSecret || 'default-secret-key', // The _secret_ to sign the JWTs | |
| getToken: getTokenFromHeader, // How to extract the JWT from the request | |
| import { expressjwt as jwt } from 'express-jwt'; | |
| import config from '../../config'; | |
| // Ensure JWT secret is configured (fail fast outside tests) | |
| if (!config.jwtSecret && process.env.NODE_ENV !== 'test') { | |
| throw new Error('JWT secret is required. Set JWT_SECRET in the environment.'); | |
| } | |
| const isAuth = jwt({ | |
| algorithms: ['HS256'], // This is the default algorithm used by jsonwebtoken | |
| secret: config.jwtSecret as string, // The _secret_ to sign the JWTs | |
| getToken: getTokenFromHeader, // How to extract the JWT from the request | |
| }); |
🤖 Prompt for AI Agents
In src/api/middlewares/isAuth.ts around lines 24-25, the code supplies a
hardcoded fallback JWT secret ('default-secret-key'); remove that fallback and
instead require config.jwtSecret to be present, throwing an error (or exiting
startup) if it's missing unless the process is running in a test environment
(e.g., NODE_ENV === 'test'); ensure getTokenFromHeader remains unchanged and
that the module fails fast at startup with a clear message about the missing JWT
secret so the app never runs with a default insecure key.
| * JWT Secret | ||
| */ | ||
| jwtSecret: process.env.JWT_SECRET, | ||
| jwtSecret: process.env.JWT_SECRET || 'default-secret-key', |
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.
Do not ship a default JWT secret.
Default secrets are a security risk. Require it in non-test envs.
- jwtSecret: process.env.JWT_SECRET || 'default-secret-key',
+ jwtSecret: process.env.JWT_SECRET as string,Add a guard (outside this hunk):
const isTest = process.env.NODE_ENV === 'test';
if (!process.env.JWT_SECRET && !isTest) {
throw new Error('JWT_SECRET is required');
}🤖 Prompt for AI Agents
In src/config/index.ts around line 20, the config currently falls back to a
hardcoded JWT default which is insecure; remove the default and require
JWT_SECRET in non-test environments by adding a guard near the top of this file
(outside the shown hunk) that checks NODE_ENV !== 'test' and throws an Error if
process.env.JWT_SECRET is missing, then use process.env.JWT_SECRET directly in
the exported config.
| port: Number.parseInt(process.env.TYPEORM_PORT!), | ||
| /** |
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.
💡 Verification agent
🧩 Analysis chain
Fail-fast env parsing for DB port; avoid non-null assertion.
Number.parseInt(process.env.TYPEORM_PORT!, 10) yields NaN if unset. Provide default or throw with a clear error.
- // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
- port: Number.parseInt(process.env.TYPEORM_PORT!),
+ port: process.env.TYPEORM_PORT
+ ? Number.parseInt(process.env.TYPEORM_PORT, 10)
+ : undefined,If port is required, add an explicit check near startup and fail with a descriptive message.
🏁 Script executed:
#!/bin/bash
set -e
echo "Occurrences of TYPEORM_PORT in src/config/index.ts:"
rg -nC3 'TYPEORM_PORT' src/config/index.ts
echo -e "\nFirst 100 lines of src/config/index.ts for context:"
sed -n '1,100p' src/config/index.ts
echo -e "\nTypeORM connection initialization sites in src/:"
rg -nC3 'createConnection' src/**/*.ts || trueLength of output: 2967
🏁 Script executed:
#!/bin/bash
set -e
echo "=== src/loaders/database.ts (first 200 lines) ==="
sed -n '1,200p' src/loaders/database.tsLength of output: 1047
Add fail-fast validation for TYPEORM_PORT
-
In src/config/index.ts, remove the non-null assertion and inline
Number.parseInt(process.env.TYPEORM_PORT!). Instead, before constructingdatabase.port, do:const rawPort = process.env.TYPEORM_PORT; if (!rawPort) throw new Error('Missing required TYPEORM_PORT environment variable'); const port = Number.parseInt(rawPort, 10); if (Number.isNaN(port)) throw new Error(`Invalid TYPEORM_PORT: ${rawPort}`); // … database: { // … port, // … },
🤖 Prompt for AI Agents
In src/config/index.ts around lines 38-39, remove the non-null assertion and
inline parse of process.env.TYPEORM_PORT; instead read process.env.TYPEORM_PORT
into a local rawPort, throw a descriptive Error if rawPort is missing, parse it
with Number.parseInt(rawPort, 10), throw a descriptive Error if the result is
NaN, and then assign the parsed numeric port value to database.port when
constructing the config.
| export class Exception extends Error { | ||
| public status!: number; | ||
| public code?: string; | ||
| constructor(message: string, status: number = 500, code?: string, link?: string) { |
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.
💡 Verification agent
🧩 Analysis chain
Prevent “/undefined” in dev link and reduce duplication.
When code is undefined, the message appends ${link}/undefined. Guard it.
- if (isDev && link) {
- message = `${message}\n> More details: ${link}/${code}`;
- }
+ if (isDev && link) {
+ const linkSuffix = code ? `/${code}` : '';
+ message = `${message}\n> More details: ${link}${linkSuffix}`;
+ }Also verify imports across the repo now use a named export:
Also applies to: 23-25
🏁 Script executed:
#!/usr/bin/env bash
rg -nP "import\s+Exception\s+from\s+['\"]\.\/?Exception" -g '!**/dist/**'Length of output: 77
🏁 Script executed:
#!/usr/bin/env bash
set -eo pipefail
echo "🔍 Searching for default ESM imports of Exception..."
rg -nP "import\s+Exception\s+from\s+['\"][^'\"]*Exception['\"]" -g '!**/dist/**' || true
echo "🔍 Searching for CommonJS require calls importing Exception..."
rg -nP "require\(['\"][^'\"]*Exception['\"]\)" -g '!**/dist/**' || trueLength of output: 443
Guard against undefined code in dev link (src/Exception/index.ts lines 12–15 and 23–25)
Replace:
- if (isDev && link) {
- message = `${message}\n> More details: ${link}/${code}`;
- }
+ if (isDev && link) {
+ const linkSuffix = code ? `/${code}` : '';
+ message = `${message}\n> More details: ${link}${linkSuffix}`;
+ }Default imports of Exception were searched for and none found—named exports are correctly used.
📝 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 class Exception extends Error { | |
| public status!: number; | |
| public code?: string; | |
| constructor(message: string, status: number = 500, code?: string, link?: string) { | |
| if (isDev && link) { | |
| const linkSuffix = code ? `/${code}` : ''; | |
| message = `${message}\n> More details: ${link}${linkSuffix}`; | |
| } |
🤖 Prompt for AI Agents
In src/Exception/index.ts around lines 12–15 and 23–25, the constructor/logic
assumes `code` is defined when building the dev link; guard against undefined by
only constructing the dev link if `code` is truthy (or provide a safe fallback
like 'unknown'), or by making the link optional. Modify the
constructor/assignment so it checks `if (code) { /* build link using code */ }
else { /* omit link or use fallback */ }` and ensure no template interpolation
uses an undefined `code`.
| @Column({ | ||
| unique: true, | ||
| nullable: false, | ||
| transformer: [lowercase], | ||
| }) | ||
| public email: string; | ||
| public email!: string; |
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.
🛠️ Refactor suggestion
Avoid locale-dependent email lowercasing.
lowercase uses toLocaleLowerCase(), which can mis-handle emails in locales like Turkish. Prefer toLowerCase() and null-safety.
Apply in ValueTransformers.ts (outside this hunk):
export const lowercase: ValueTransformer = {
- to: (entityValue: string) => {
- return entityValue.toLocaleLowerCase();
- },
+ to: (entityValue: string) => entityValue?.toLowerCase(),
from: (databaseValue: string) => {
return databaseValue;
},
};📝 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.
| @Column({ | |
| unique: true, | |
| nullable: false, | |
| transformer: [lowercase], | |
| }) | |
| public email: string; | |
| public email!: string; | |
| // src/transformers/ValueTransformers.ts | |
| export const lowercase: ValueTransformer = { | |
| to: (entityValue: string) => entityValue?.toLowerCase(), | |
| from: (databaseValue: string) => { | |
| return databaseValue; | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In src/models/User.ts around lines 26-31 the email column uses a transformer
named `lowercase` which currently relies on locale-dependent
toLocaleLowerCase(); update the transformer implementation in
ValueTransformers.ts to use a null-safe, locale-independent conversion (e.g.,
return value == null ? value : String(value).toLowerCase()) and export/rename or
add it as `safeLowercase`; then update the model to use that transformer
(replace `lowercase` with the new `safeLowercase`) so emails are lowercased
deterministically and safely.
| } catch (error: any) { | ||
| if (error.name === 'MongoError' && error.code === 11000) { | ||
| // Duplicate username | ||
| throw new Error('User already exist!'); | ||
| } | ||
| console.log(error); | ||
| console.error(error); | ||
| throw 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.
🛠️ Refactor suggestion
Duplicate-user detection is brittle on modern Mongo/TypeORM; broaden detection and avoid leaking raw errors.
MongoError → MongoServerError (code 11000) in newer drivers; SQL backends use different codes (23505, ER_DUP_ENTRY). Prefer throwing domain Exception with 409.
Apply:
- } catch (error: any) {
- if (error.name === 'MongoError' && error.code === 11000) {
- // Duplicate username
- throw new Error('User already exist!');
- }
- console.error(error);
- throw error;
+ } catch (err) {
+ const code = (err as any)?.code ?? (err as any)?.driverError?.code;
+ if (code === 11000 || code === '23505' || code === 'ER_DUP_ENTRY') {
+ throw new Exception('User already exists', 409);
+ }
+ console.error(err);
+ throw err;
}📝 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.
| } catch (error: any) { | |
| if (error.name === 'MongoError' && error.code === 11000) { | |
| // Duplicate username | |
| throw new Error('User already exist!'); | |
| } | |
| console.log(error); | |
| console.error(error); | |
| throw error; | |
| } | |
| } catch (err) { | |
| const code = (err as any)?.code ?? (err as any)?.driverError?.code; | |
| if (code === 11000 || code === '23505' || code === 'ER_DUP_ENTRY') { | |
| throw new Exception('User already exists', 409); | |
| } | |
| console.error(err); | |
| throw err; | |
| } |
🤖 Prompt for AI Agents
In src/services/auth.ts around lines 46 to 53, the catch block currently only
checks for MongoError code 11000 and prints the raw error; update it to robustly
detect duplicate-entity errors across drivers and return a domain-level 409
error without leaking internals: check for MongoServerError name or code ===
11000, Postgres unique-violation code '23505', MySQL 'ER_DUP_ENTRY' (or numeric
code 1062), and also inspect error.code, error.errno, error.driverError?.code
and error.name; when any of these indicate a duplicate key/unique constraint
violation, throw a new domain Exception (or Error subclass) with a clear message
like "User already exists" and HTTP status 409, otherwise log a sanitized
message (no full stack/driver payload) and rethrow or wrap the error for
upstream handling.
Migrate project to latest versions and modernize codebase to update deprecated tools, improve type safety, and ensure compatibility with modern Node.js environments.
This PR replaces TSLint with ESLint, upgrades Node.js to the latest LTS (>=18.0.0), and updates TypeScript to v5.x with strict mode enabled. It also includes comprehensive dependency updates, modernizes import syntax, and resolves various type and linting issues.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Tests
Chores