From 09d0bbf1b4d27b4304922bc796d11f193e0d4968 Mon Sep 17 00:00:00 2001
From: Yusuke Wada <yusuke@kamawada.com>
Date: Wed, 21 Aug 2024 10:38:03 +0900
Subject: [PATCH] fix(client): replace optional params to url correctly

---
 src/client/client.test.ts | 15 ++++++++++++++-
 src/client/utils.test.ts  | 11 +++++++++++
 src/client/utils.ts       |  6 +++---
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/client/client.test.ts b/src/client/client.test.ts
index 5e0b5218f..9f0add03f 100644
--- a/src/client/client.test.ts
+++ b/src/client/client.test.ts
@@ -815,7 +815,9 @@ describe('Infer the response type with different status codes', () => {
 })
 
 describe('$url() with a param option', () => {
-  const app = new Hono().get('/posts/:id/comments', (c) => c.json({ ok: true }))
+  const app = new Hono()
+    .get('/posts/:id/comments', (c) => c.json({ ok: true }))
+    .get('/something/:firstId/:secondId/:version?', (c) => c.json({ ok: true }))
   type AppType = typeof app
   const client = hc<AppType>('http://localhost')
 
@@ -832,6 +834,17 @@ describe('$url() with a param option', () => {
     const url = client.posts[':id'].comments.$url()
     expect(url.pathname).toBe('/posts/:id/comments')
   })
+
+  it('Should return the correct path - /something/123/456', async () => {
+    const url = client.something[':firstId'][':secondId'][':version?'].$url({
+      param: {
+        firstId: '123',
+        secondId: '456',
+        version: undefined,
+      },
+    })
+    expect(url.pathname).toBe('/something/123/456')
+  })
 })
 
 describe('Client can be awaited', () => {
diff --git a/src/client/utils.test.ts b/src/client/utils.test.ts
index baaa82d2e..7adc6b00b 100644
--- a/src/client/utils.test.ts
+++ b/src/client/utils.test.ts
@@ -46,6 +46,17 @@ describe('replaceUrlParams', () => {
     const replacedUrl = replaceUrlParam(url, params)
     expect(replacedUrl).toBe('http://localhost/year/2024/month/2')
   })
+
+  it('Should replace correctly when it has optional parameters', () => {
+    const url = 'http://localhost/something/:firstId/:secondId/:version?'
+    const params = {
+      firstId: '123',
+      secondId: '456',
+      version: undefined,
+    }
+    const replacedUrl = replaceUrlParam(url, params)
+    expect(replacedUrl).toBe('http://localhost/something/123/456')
+  })
 })
 
 describe('replaceUrlProtocol', () => {
diff --git a/src/client/utils.ts b/src/client/utils.ts
index 31c287cc6..591b7bc76 100644
--- a/src/client/utils.ts
+++ b/src/client/utils.ts
@@ -7,10 +7,10 @@ export const mergePath = (base: string, path: string) => {
   return base + path
 }
 
-export const replaceUrlParam = (urlString: string, params: Record<string, string>) => {
+export const replaceUrlParam = (urlString: string, params: Record<string, string | undefined>) => {
   for (const [k, v] of Object.entries(params)) {
-    const reg = new RegExp('/:' + k + '(?:{[^/]+})?')
-    urlString = urlString.replace(reg, `/${v}`)
+    const reg = new RegExp('/:' + k + '(?:{[^/]+})?\\??')
+    urlString = urlString.replace(reg, v ? `/${v}` : '')
   }
   return urlString
 }