Feat: Completely migrate Node.js Express backend to TypeScript (Fixes #853)#978
Feat: Completely migrate Node.js Express backend to TypeScript (Fixes #853)#978ArshVermaGit wants to merge 3 commits into
Conversation
|
@ArshVermaGit is attempting to deploy a commit to the durdana3105's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds ChangesBackend TypeScript Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ArshVermaGit
left a comment
ArshVermaGit
left a comment
There was a problem hiding this comment.
issue resolved
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/controllers/cronController.ts (1)
69-71:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Undefined variable
claimError— should beerror.Line 63-67 destructures
errorfrom the Supabase query, but line 69 checksif (claimError). The variableclaimErroris undefined, so this error handling will never trigger even when the query fails, causing downstream crashes when the code assumes a successful query.🔧 Proposed fix
const { data: notifications, error } = await supabase .from("notifications") .update({ push_claimed_at: claimedAt }) .is("push_claimed_at", null) .select("id,user_id,title,body,action_url"); - if (claimError) { - return res.status(500).json({ error: claimError.message }); + if (error) { + return res.status(500).json({ error: error.message }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/controllers/cronController.ts` around lines 69 - 71, The variable name mismatch in the error handling block is preventing error detection. The Supabase query destructures a variable named error (lines 63-67), but the conditional at line 69 incorrectly references claimError which is undefined. Replace claimError with error in the if condition to ensure error handling actually triggers when the query fails.
🧹 Nitpick comments (6)
backend/validation/schemas.ts (1)
1-1: 🏗️ Heavy liftTrack removal of
@ts-nocheckfor Zod schema validation.The
@ts-nocheckdirective disables type checking for Zod schemas, which have excellent TypeScript support. Removing this directive would enable TypeScript to validate schema definitions and catch type mismatches between schemas and runtime usage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/validation/schemas.ts` at line 1, Remove the `@ts-nocheck` directive from the top of the file in backend/validation/schemas.ts to enable TypeScript type checking for the Zod schema definitions. After removing the directive, verify that all Zod schema definitions have proper type annotations and that TypeScript compilation succeeds without errors. This will ensure that schema definitions are validated at compile time and catch any type mismatches between the schemas and their runtime usage.backend/middlewares/rateLimiter.ts (2)
41-41: ⚡ Quick winReplace
anywith a proper options interface.The
options: anyannotation defeats TypeScript's type safety. Based on usage at lines 42-44, define an interface:✨ Suggested type definition
+interface RateLimiterOptions { + windowMs?: number; + maxRequests?: number; + maxEntries?: number; +} + -export const createRateLimiter = (options: any = {}) => { +export const createRateLimiter = (options: RateLimiterOptions = {}) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/middlewares/rateLimiter.ts` at line 41, The createRateLimiter function parameter uses `any` type which removes TypeScript type safety. Define a proper interface (e.g., RateLimiterOptions) that describes the expected properties based on how the options object is used in lines 42-44, then replace the `any` type annotation with this interface name in the createRateLimiter function signature.
45-45: ⚡ Quick winReplace
Map<string, any>with a properly typed entry structure.Based on the entry structure at lines 73-74, the Map value should be typed as
{ count: number; windowStart: number }.✨ Suggested type definition
+interface RateLimitEntry { + count: number; + windowStart: number; +} + - const store = new Map<string, any>(); + const store = new Map<string, RateLimitEntry>();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/middlewares/rateLimiter.ts` at line 45, The `store` variable at line 45 is declared with `Map<string, any>()` which uses an imprecise type. Replace the `any` type with the proper entry structure based on the usage at lines 73-74. The Map value type should be `{ count: number; windowStart: number }` to accurately represent the count and windowStart properties that are being stored and accessed throughout the rate limiter implementation.backend/app.ts (1)
66-66: ⚡ Quick winReplace
req: anywith proper Express types.Using
anydefeats TypeScript's type safety. Import and use Express'sRequest,Response, andNextFunctiontypes. For the customrequestIdproperty, augment the Express Request interface.✨ Suggested proper typing
Add type augmentation at the top of the file or in a separate types file:
import { Request, Response, NextFunction } from 'express'; declare global { namespace Express { interface Request { requestId?: string; } } }Then update the middleware:
-app.use((req: any, res, next) => { +app.use((req: Request, res: Response, next: NextFunction) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app.ts` at line 66, Replace the `req: any` type annotation in the app.use middleware function with proper Express types to maintain type safety. Import Request, Response, and NextFunction from the 'express' package, then declare a global type augmentation for the Express namespace to add the custom requestId property to the Request interface. Finally, update the middleware function signature to use (req: Request, res: Response, next: NextFunction) instead of (req: any, res, next) so TypeScript can properly type-check the middleware parameters and the custom requestId property will be recognized as a valid Request property.backend/tsconfig.json (1)
8-9: 🏗️ Heavy liftTrack re-enabling strict mode as follow-up work.
The PR objectives emphasize "type-safe backend code that prevents runtime logic bugs," but
strict: falseandnoImplicitAny: falseprovide minimal type safety. While this is acceptable for an incremental migration, the configuration should eventually enable strict mode to realize the full benefits of TypeScript.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tsconfig.json` around lines 8 - 9, Add a comment in the tsconfig.json file near the strict mode settings (the `strict: false` and `noImplicitAny: false` lines) to document that re-enabling strict mode is tracked as follow-up work. The comment should reference the need to eventually enable these strict type-checking options to achieve full type safety and prevent runtime logic bugs, and optionally reference any tracking system (issue tracker, etc.) being used to monitor this work.backend/controllers/cronController.ts (1)
1-1: 🏗️ Heavy liftTrack removal of
@ts-nocheckas follow-up work.The
@ts-nocheckdirective disables all TypeScript type checking for this file, preventing the type-safety benefits mentioned in the PR objectives. While acceptable for incremental migration, plan to add proper types and remove this directive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/controllers/cronController.ts` at line 1, The `@ts-nocheck` directive at the beginning of the cronController.ts file disables TypeScript type checking for the entire file, which undermines type safety. Remove the `@ts-nocheck` comment from the first line and add proper TypeScript type annotations throughout the file (such as explicit parameter types, return types for functions, and proper typing for variables and data structures) to ensure full type-safety coverage and align with the PR objectives of maintaining type safety in the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/controllers/aiController.ts`:
- Around line 109-111: The code is using camelCase `responseFormat` to assign
the value to the request body object, but OpenRouter's Chat Completions API
expects the snake_case parameter name `response_format`. Update the assignment
in the if block (where responseFormat is checked and assigned to body) to use
the correct snake_case field name `response_format` instead of camelCase
`responseFormat`.
In `@package.json`:
- Around line 102-103: The `@types/express` package is pinned to version 5.0.6,
which provides type definitions for Express 5, but the express package itself is
at version 4.21.2. This version mismatch causes TypeScript compilation errors in
middleware and route handlers due to incompatible type signatures. Update the
`@types/express` version in package.json to use a version compatible with Express
4 (such as ^4.17.x) to align the type definitions with the actual Express 4
runtime being used.
---
Outside diff comments:
In `@backend/controllers/cronController.ts`:
- Around line 69-71: The variable name mismatch in the error handling block is
preventing error detection. The Supabase query destructures a variable named
error (lines 63-67), but the conditional at line 69 incorrectly references
claimError which is undefined. Replace claimError with error in the if condition
to ensure error handling actually triggers when the query fails.
---
Nitpick comments:
In `@backend/app.ts`:
- Line 66: Replace the `req: any` type annotation in the app.use middleware
function with proper Express types to maintain type safety. Import Request,
Response, and NextFunction from the 'express' package, then declare a global
type augmentation for the Express namespace to add the custom requestId property
to the Request interface. Finally, update the middleware function signature to
use (req: Request, res: Response, next: NextFunction) instead of (req: any, res,
next) so TypeScript can properly type-check the middleware parameters and the
custom requestId property will be recognized as a valid Request property.
In `@backend/controllers/cronController.ts`:
- Line 1: The `@ts-nocheck` directive at the beginning of the cronController.ts
file disables TypeScript type checking for the entire file, which undermines
type safety. Remove the `@ts-nocheck` comment from the first line and add proper
TypeScript type annotations throughout the file (such as explicit parameter
types, return types for functions, and proper typing for variables and data
structures) to ensure full type-safety coverage and align with the PR objectives
of maintaining type safety in the codebase.
In `@backend/middlewares/rateLimiter.ts`:
- Line 41: The createRateLimiter function parameter uses `any` type which
removes TypeScript type safety. Define a proper interface (e.g.,
RateLimiterOptions) that describes the expected properties based on how the
options object is used in lines 42-44, then replace the `any` type annotation
with this interface name in the createRateLimiter function signature.
- Line 45: The `store` variable at line 45 is declared with `Map<string, any>()`
which uses an imprecise type. Replace the `any` type with the proper entry
structure based on the usage at lines 73-74. The Map value type should be `{
count: number; windowStart: number }` to accurately represent the count and
windowStart properties that are being stored and accessed throughout the rate
limiter implementation.
In `@backend/tsconfig.json`:
- Around line 8-9: Add a comment in the tsconfig.json file near the strict mode
settings (the `strict: false` and `noImplicitAny: false` lines) to document that
re-enabling strict mode is tracked as follow-up work. The comment should
reference the need to eventually enable these strict type-checking options to
achieve full type safety and prevent runtime logic bugs, and optionally
reference any tracking system (issue tracker, etc.) being used to monitor this
work.
In `@backend/validation/schemas.ts`:
- Line 1: Remove the `@ts-nocheck` directive from the top of the file in
backend/validation/schemas.ts to enable TypeScript type checking for the Zod
schema definitions. After removing the directive, verify that all Zod schema
definitions have proper type annotations and that TypeScript compilation
succeeds without errors. This will ensure that schema definitions are validated
at compile time and catch any type mismatches between the schemas and their
runtime usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c1eabef1-df50-443a-8545-a5c8ceb74ba5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
backend/app.tsbackend/config.tsbackend/controllers/aiController.tsbackend/controllers/cronController.tsbackend/controllers/matchController.tsbackend/controllers/notificationController.tsbackend/middlewares/errorHandler.tsbackend/middlewares/rateLimiter.tsbackend/middlewares/requireAuth.tsbackend/middlewares/requireCronSecret.tsbackend/middlewares/validate.tsbackend/routers/aiRoutes.tsbackend/routers/authRoutes.tsbackend/routers/chatRoutes.tsbackend/routers/cronRoutes.tsbackend/routers/matchRoutes.tsbackend/routers/notificationRoutes.tsbackend/routes/users.tsbackend/server.tsbackend/tests/aiController.test.tsbackend/tests/aiRobustness.test.tsbackend/tests/cronController.test.tsbackend/tests/dispatchPushNotifications.test.tsbackend/tests/docs.test.tsbackend/tests/matchController.test.tsbackend/tests/requireAuth.test.tsbackend/tests/setup.tsbackend/tests/studyRooms.integration.test.tsbackend/tests/studyRooms.test.tsbackend/tests/validation.test.tsbackend/tsconfig.jsonbackend/utils/asyncHandler.tsbackend/utils/env.tsbackend/utils/httpError.tsbackend/utils/sendEmail.tsbackend/utils/skillGraph.tsbackend/utils/supabase.tsbackend/validation/schemas.tspackage.json
|
please resolve merge conflicts |
2 participants
This PR permanently resolves issue #853 by unifying our entire stack into the TypeScript ecosystem, eliminating the technical debt caused by our fragmented language architectures. It fully migrates all 37 backend .js files to strict .ts and integrates a new fully-configured backend/tsconfig.json utilizing modern ESM (NodeNext) configuration. Additionally, package.json now relies on tsx for seamless and high-velocity server execution (via "server:start" and "server:dev"), preventing easily preventable runtime logic bugs and laying down the final foundation needed for our frontend to safely share type definitions and DTO contracts with the backend APIs.
Closes #853
Summary by CodeRabbit
responseFormatis sent in the expected camelCase form, improving response formatting compatibility.tsx), bumped TypeScript, and adjusted server dependency/type packages.