fix(request): return string | undefined from param() when path type is any#4723
fix(request): return string | undefined from param() when path type is any#4723yusukebe merged 4 commits intohonojs:mainfrom
string | undefined from param() when path type is any#4723Conversation
When middleware uses Context with a default path type parameter (any),
c.req.param('key') incorrectly returns string instead of string | undefined.
This is because ParamKeys<any> resolves to string, matching the first
overload that returns string.
Add a new overload that detects when P is any using the 0 extends (1 & P)
trick and returns string | undefined, ensuring type safety in middleware.
Fixes honojs#3198
yusukebe
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4723 +/- ##
==========================================
+ Coverage 91.43% 91.46% +0.03%
==========================================
Files 173 177 +4
Lines 11382 11563 +181
Branches 3300 3360 +60
==========================================
+ Hits 10407 10576 +169
- Misses 974 986 +12
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Is this really going to fix the issue written in #3198 (comment)? Can you write a test and confirm this PR fixes it? |
Added two type-level tests to src/request.test.ts in the Param > Type describe block:
|
This comment was marked as off-topic.
This comment was marked as off-topic.
yusukebe
changed the title
string from param() when path type is any
yusukebe
changed the title
string from param() when path type is anystring | undefined from param() when path type is any
| expectTypeOf(foo).toEqualTypeOf<number>() | ||
| const id = c.req.param('id') | ||
| expectTypeOf(id).toEqualTypeOf<string>() | ||
| expectTypeOf(id).toEqualTypeOf<string | undefined>() |
There was a problem hiding this comment.
There may be some discussion about updating the test, but if we accept this PR, the old test is wrong. The value of c.req.param('id') can be string | undefined.
|
I updated the type definition and the existing test. I think the behavior is good. I'll merge this. Thank you for your help. |
|
@andrewdamelio Shouldn't the import { Hono } from 'hono'
import { upgradeWebSocket } from "hono/bun";
const app = new Hono()
app.get('/rooms/:roomId/ws', upgradeWebSocket((c) => {
const roomId = c.req.param("roomId");
return {
onOpen() {
console.log('WebSocket opened')
},
}
})); |
4 participants
When middleware uses Context with a default path type parameter (any), c.req.param('key') incorrectly returns string instead of string | undefined. This is because ParamKeys resolves to string, matching the first overload that returns string.
Add a new overload that detects when P is any using the 0 extends (1 & P) trick and returns string | undefined, ensuring type safety in middleware.
Fixes #3198
The author should do the following, if applicable
bun run format:fix && bun run lint:fixto format the code