Skip to content
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

RPC client doesn't handle optional path parameters #3303

Closed
cojo opened this issue Aug 20, 2024 · 2 comments · Fixed by #3304
Closed

RPC client doesn't handle optional path parameters #3303

cojo opened this issue Aug 20, 2024 · 2 comments · Fixed by #3304
Labels

Comments

@cojo
Copy link

cojo commented Aug 20, 2024

What version of Hono are you using?

4.5.6

What runtime/platform is your app running on?

Cloudflare Pages / Workers

What steps can reproduce the bug?

  1. Make an endpoint with an optional path param, e.g. /something/:firstId/:secondId/:version?
  2. Try to call the endpoint without the optional parameter using the Hono RPC client, e.g. client.something[":firstId"][":secondId"][":version?"].$get

What is the expected behavior?

We would expect it to call e.g. GET /something/first/second.

What do you see instead?

It instead calls either GET /something/first/second/:version, GET /something/first/second/undefined, or GET /something/first/second/null, depending on how you try to avoid the optional parameter.

Additional information

Based on my analysis of https://github.com/honojs/hono/blob/main/src/client/client.ts I believe this functionality was simply forgotten / not handled (I do not see any handling of undefined pathParams currently). Seems like an oversight? But we would definitely like to utilize this since Hono supports it on the router / server side.

Thank you in advance!

@cojo cojo added the triage label Aug 20, 2024
@yusukebe
Copy link
Member

Hi @cojo

This is a bug! Thank you for rasing the issue. I'll try to fix it.

@cojo
Copy link
Author

cojo commented Aug 21, 2024

Awesome, thanks for the quick fix @yusukebe ! Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants