fix: preserve trailing optional path parameters (#2169)

Co-authored-by: Kai Spencer <51139521+KaiSpencer@users.noreply.github.com>

Author: kettanaito

src/core/utils/matching/matchRequestUrl.test.ts

@@ -61,6 +61,17 @@ describe('matchRequestUrl', () => {
     expect(match).toHaveProperty('matches', false)
     expect(match).toHaveProperty('params', {})
   })
+
+  test('returns true when matching optional path parameters', () => {
+    const match = matchRequestUrl(
+      new URL('https://test.mswjs.io/user'),
+      'https://test.mswjs.io/user/:userId?',
+    )
+    expect(match).toHaveProperty('matches', true)
+    expect(match).toHaveProperty('params', {
+      userId: undefined,
+    })
+  })
 })
 
 describe('coercePath', () => {

src/core/utils/matching/normalizePath.test.ts

@@ -43,8 +43,14 @@ test('returns a path pattern string as-is', () => {
   expect(normalizePath('*/resource/*')).toEqual('*/resource/*')
 })
 
-test('removeß query parameters and hashes from a path pattern string', () => {
+test('removes query parameters and hashes from a path pattern string', () => {
   expect(normalizePath(':api/user?query=123#some')).toEqual(
     'http://localhost/:api/user',
   )
 })
+
+test('preserves optional path parameters', () => {
+  expect(normalizePath('/user/:userId?')).toEqual(
+    'http://localhost/user/:userId?',
+  )
+})

src/core/utils/matching/normalizePath.ts

@@ -8,6 +8,7 @@ import { getAbsoluteUrl } from '../url/getAbsoluteUrl'
  * - Removes query parameters and hashes.
  * - Rebases relative URLs against the "baseUrl" or the current location.
  * - Preserves relative URLs in Node.js, unless specified otherwise.
+ * - Preserves optional path parameters.
  */
 export function normalizePath(path: Path, baseUrl?: string): Path {
   // RegExp paths do not need normalization.

src/core/utils/url/cleanUrl.test.ts

@@ -1,17 +1,22 @@
 import { cleanUrl } from './cleanUrl'
 
-test('removes query parameters from a URL string', () => {
+it('removes query parameters from a URL string', () => {
   expect(cleanUrl('/user?id=123')).toEqual('/user')
   expect(cleanUrl('/user?id=123&id=456')).toEqual('/user')
   expect(cleanUrl('/user?id=123&role=admin')).toEqual('/user')
 })
 
-test('removes hashes from a URL string', () => {
+it('removes hashes from a URL string', () => {
   expect(cleanUrl('/user#hash')).toEqual('/user')
   expect(cleanUrl('/user#hash-with-dashes')).toEqual('/user')
 })
 
-test('removes both query parameters and hashes from a URL string', () => {
+it('removes both query parameters and hashes from a URL string', () => {
   expect(cleanUrl('/user?id=123#some')).toEqual('/user')
   expect(cleanUrl('/user?id=123&role=admin#some')).toEqual('/user')
 })
+
+it('preserves optional path parameters', () => {
+  expect(cleanUrl('/user/:id?')).toEqual('/user/:id?')
+  expect(cleanUrl('/user/:id?/:messageId?')).toEqual('/user/:id?/:messageId?')
+})

src/core/utils/url/cleanUrl.ts

@@ -5,8 +5,16 @@ export function getSearchParams(path: string) {
 }
 
 /**
- * Removes query parameters and hashes from a given URL string.
+ * Removes search parameters and the fragment
+ * from a given URL string.
  */
 export function cleanUrl(path: string): string {
+  // If the path ends with an optional path parameter,
+  // return it as-is.
+  if (path.endsWith('?')) {
+    return path
+  }
+
+  // Otherwise, remove the search and fragment from it.
   return path.replace(REDUNDANT_CHARACTERS_EXP, '')
 }

src/core/utils/url/getAbsoluteUrl.node.test.ts

@@ -3,16 +3,16 @@
  */
 import { getAbsoluteUrl } from './getAbsoluteUrl'
 
-test('returns a given relative URL as-is', () => {
+it('returns a given relative URL as-is', () => {
   expect(getAbsoluteUrl('/reviews')).toBe('/reviews')
 })
 
-test('rebases a relative URL against a custom base URL', () => {
+it('rebases a relative URL against a custom base URL', () => {
   expect(getAbsoluteUrl('/user', 'https://api.github.com')).toEqual(
     'https://api.github.com/user',
   )
 })
-test('returns a given absolute URL as-is', () => {
+it('returns a given absolute URL as-is', () => {
   expect(getAbsoluteUrl('https://api.mswjs.io/users')).toBe(
     'https://api.mswjs.io/users',
   )

src/core/utils/url/getAbsoluteUrl.test.ts

@@ -3,27 +3,27 @@
  */
 import { getAbsoluteUrl } from './getAbsoluteUrl'
 
-test('rebases a relative URL against the current "baseURI" (default)', () => {
+it('rebases a relative URL against the current "baseURI" (default)', () => {
   expect(getAbsoluteUrl('/reviews')).toEqual('http://localhost/reviews')
 })
 
-test('rebases a relative URL against a custom base URL', () => {
+it('rebases a relative URL against a custom base URL', () => {
   expect(getAbsoluteUrl('/user', 'https://api.github.com')).toEqual(
     'https://api.github.com/user',
   )
 })
 
-test('returns a given absolute URL as-is', () => {
+it('returns a given absolute URL as-is', () => {
   expect(getAbsoluteUrl('https://api.mswjs.io/users')).toEqual(
     'https://api.mswjs.io/users',
   )
 })
 
-test('returns an absolute URL given a relative path without a leading slash', () => {
+it('returns an absolute URL given a relative path without a leading slash', () => {
   expect(getAbsoluteUrl('users')).toEqual('http://localhost/users')
 })
 
-test('returns a path with a pattern as-is', () => {
+it('returns a path with a pattern as-is', () => {
   expect(getAbsoluteUrl(':api/user')).toEqual('http://localhost/:api/user')
   expect(getAbsoluteUrl('*/resource/*')).toEqual('*/resource/*')
 })

src/core/utils/url/isAbsoluteUrl.test.ts

@@ -3,30 +3,30 @@
  */
 import { isAbsoluteUrl } from './isAbsoluteUrl'
 
-test('returns true for the "http" scheme', () => {
+it('returns true for the "http" scheme', () => {
   expect(isAbsoluteUrl('http://www.domain.com')).toEqual(true)
 })
 
-test('returns true for the "https" scheme', () => {
+it('returns true for the "https" scheme', () => {
   expect(isAbsoluteUrl('https://www.domain.com')).toEqual(true)
 })
 
-test('returns true for the "ws" scheme', () => {
+it('returns true for the "ws" scheme', () => {
   expect(isAbsoluteUrl('ws://www.domain.com')).toEqual(true)
 })
 
-test('returns true for the "ftp" scheme', () => {
+it('returns true for the "ftp" scheme', () => {
   expect(isAbsoluteUrl('ftp://www.domain.com')).toEqual(true)
 })
 
-test('returns true for the custom scheme', () => {
+it('returns true for the custom scheme', () => {
   expect(isAbsoluteUrl('web+my://www.example.com')).toEqual(true)
 })
 
-test('returns false for the relative URL', () => {
+it('returns false for the relative URL', () => {
   expect(isAbsoluteUrl('/test')).toEqual(false)
 })
 
-test('returns false for the relative URL without a leading slash', () => {
+it('returns false for the relative URL without a leading slash', () => {
   expect(isAbsoluteUrl('test')).toEqual(false)
 })

test/node/rest-api/request/matching/path-params-optional.node.test.ts

@@ -0,0 +1,43 @@
+/**
+ * @vitest-environment node
+ */
+import { HttpResponse, http } from 'msw'
+import { setupServer } from 'msw/node'
+
+const server = setupServer()
+
+beforeAll(() => {
+  server.listen()
+})
+
+afterEach(() => {
+  server.resetHandlers()
+})
+
+afterAll(() => {
+  server.close()
+})
+
+it('intercepts the request that fully matches the path', async () => {
+  server.use(
+    http.get('http://localhost/user/:id?', () =>
+      HttpResponse.json({ mocked: true }),
+    ),
+  )
+
+  const response = await fetch('http://localhost/user/123')
+  expect(response.status).toBe(200)
+  expect(await response.json()).toEqual({ mocked: true })
+})
+
+it('intercepts the request that partially matches the path', async () => {
+  server.use(
+    http.get('http://localhost/user/:id?', () =>
+      HttpResponse.json({ mocked: true }),
+    ),
+  )
+
+  const response = await fetch('http://localhost/user')
+  expect(response.status).toBe(200)
+  expect(await response.json()).toEqual({ mocked: true })
+})