fix(server): handle Express 5 req.params type union #729
Conversation
Express 5 changed the type of req.params values from `string` to `string | string[]` to account for array parameter patterns. This causes TypeScript errors when destructuring and passing to functions expecting `string`. The fix explicitly accesses and asserts the type for each parameter, which is safe because these routes use simple `:param` patterns (not array patterns like `:param+` or `:param*`). Affected files: - sessions/routes/archive.ts - sessions/routes/delete.ts - sessions/routes/unarchive.ts - sessions/routes/update.ts - terminal/routes/session-delete.ts - terminal/routes/session-resize.ts
Summary of ChangesHello @itsmylife44, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical type compatibility issue arising from the upgrade to Express 5, where the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughRefactoring parameter extraction in 6 route handlers from destructuring to explicit property access with TypeScript type assertions, consistently enforcing string types for Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a TypeScript compilation issue with Express 5 by changing how request parameters are accessed. The use of as string type assertions is a quick fix, but it's not fully type-safe. I've suggested a more robust approach using runtime checks to validate the parameter type. This ensures that if the parameter is not a string (e.g., it's undefined or an array), the server responds with a 400 Bad Request instead of potentially causing a runtime error or unexpected behavior. This improvement has been suggested for all modified route handlers.
| return async (req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const { sessionId } = req.params; | ||
| const sessionId = req.params.sessionId as string; |
There was a problem hiding this comment.
Using a type assertion with as string bypasses TypeScript's type safety and can hide runtime errors if req.params.sessionId is not a string (e.g., undefined or string[]). A runtime check is a more robust approach. This ensures the input is validated and invalid requests are handled gracefully with a 400 Bad Request response.
const sessionId = req.params.sessionId;
if (typeof sessionId !== 'string') {
res.status(400).json({ success: false, error: 'Invalid sessionId parameter' });
return;
}| return async (req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const { sessionId } = req.params; | ||
| const sessionId = req.params.sessionId as string; |
There was a problem hiding this comment.
Using a type assertion with as string bypasses TypeScript's type safety and can hide runtime errors if req.params.sessionId is not a string (e.g., undefined or string[]). A runtime check is a more robust approach. This ensures the input is validated and invalid requests are handled gracefully with a 400 Bad Request response.
const sessionId = req.params.sessionId;
if (typeof sessionId !== 'string') {
res.status(400).json({ success: false, error: 'Invalid sessionId parameter' });
return;
}| return async (req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const { sessionId } = req.params; | ||
| const sessionId = req.params.sessionId as string; |
There was a problem hiding this comment.
Using a type assertion with as string bypasses TypeScript's type safety and can hide runtime errors if req.params.sessionId is not a string (e.g., undefined or string[]). A runtime check is a more robust approach. This ensures the input is validated and invalid requests are handled gracefully with a 400 Bad Request response.
const sessionId = req.params.sessionId;
if (typeof sessionId !== 'string') {
res.status(400).json({ success: false, error: 'Invalid sessionId parameter' });
return;
}| return async (req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const { sessionId } = req.params; | ||
| const sessionId = req.params.sessionId as string; |
There was a problem hiding this comment.
Using a type assertion with as string bypasses TypeScript's type safety and can hide runtime errors if req.params.sessionId is not a string (e.g., undefined or string[]). A runtime check is a more robust approach. This ensures the input is validated and invalid requests are handled gracefully with a 400 Bad Request response.
const sessionId = req.params.sessionId;
if (typeof sessionId !== 'string') {
res.status(400).json({ success: false, error: 'Invalid sessionId parameter' });
return;
}| return (req: Request, res: Response): void => { | ||
| const terminalService = getTerminalService(); | ||
| const { id } = req.params; | ||
| const id = req.params.id as string; |
There was a problem hiding this comment.
Using a type assertion with as string bypasses TypeScript's type safety and can hide runtime errors if req.params.id is not a string (e.g., undefined or string[]). A runtime check is a more robust approach. This ensures the input is validated and invalid requests are handled gracefully with a 400 Bad Request response.
const id = req.params.id;
if (typeof id !== 'string') {
res.status(400).json({ success: false, error: 'Invalid id parameter' });
return;
}| return (req: Request, res: Response): void => { | ||
| const terminalService = getTerminalService(); | ||
| const { id } = req.params; | ||
| const id = req.params.id as string; |
There was a problem hiding this comment.
Using a type assertion with as string bypasses TypeScript's type safety and can hide runtime errors if req.params.id is not a string (e.g., undefined or string[]). A runtime check is a more robust approach. This ensures the input is validated and invalid requests are handled gracefully with a 400 Bad Request response.
const id = req.params.id;
if (typeof id !== 'string') {
res.status(400).json({ success: false, error: 'Invalid id parameter' });
return;
}1 participant
Summary
Fixes TypeScript compilation errors caused by Express 5's changed
req.paramstype signature.Problem
Express 5 changed
req.paramsvalues fromstringtostring | string[]to support array parameter patterns (:param+,:param*). This causes TypeScript errors:Solution
Explicitly access and assert the parameter type instead of using object destructuring:
This is safe because these routes use simple
:parampatterns, not array patterns.Changes
apps/server/src/routes/sessions/routes/archive.tsapps/server/src/routes/sessions/routes/delete.tsapps/server/src/routes/sessions/routes/unarchive.tsapps/server/src/routes/sessions/routes/update.tsapps/server/src/routes/terminal/routes/session-delete.tsapps/server/src/routes/terminal/routes/session-resize.tsTesting
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.