fix(client): replace optional params to url correctly (#3304)

yusukebe · web-flow · commit 331b3d8a96ea · 2024-08-21T10:53:54.000+09:00
diff --git 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
@@ -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
@@ -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
 }

Original file line number	Diff line number	Diff line change
`@@ -7,10 +7,10 @@ export const mergePath = (base: string, path: string) => {`
`7`	`7`	`return base + path`
`8`	`8`	`}`
`9`	`9`
`10`		`-export const replaceUrlParam = (urlString: string, params: Record<string, string>) => {`
	`10`	`+export const replaceUrlParam = (urlString: string, params: Record<string, string \| undefined>) => {`
`11`	`11`	`for (const [k, v] of Object.entries(params)) {`
`12`		`- const reg = new RegExp('/:' + k + '(?:{[^/]+})?')`
`13`		- urlString = urlString.replace(reg, `/${v}`)
	`12`	`+ const reg = new RegExp('/:' + k + '(?:{[^/]+})?\\??')`
	`13`	+ urlString = urlString.replace(reg, v ? `/${v}` : '')
`14`	`14`	`}`
`15`	`15`	`return urlString`
`16`	`16`	`}`