fix: sound URL pathname resolution implementation (#1347)

vike/shared/route/resolveRedirects.spec.ts

@@ -19,6 +19,8 @@ describe('resolveRouteStringRedirect', () => {
     expect(resolveRouteStringRedirect('/@a/@b', '/c', '/b')).toEqual(null)
     // https://github.com/vikejs/vike/issues/1347
     expect(resolveRouteStringRedirect('/npm/*', 'https://cdn.jsdelivr.net/npm/*', '/npm/@my-team/my-package')).toEqual('https://cdn.jsdelivr.net/npm/@my-team/my-package')
+    expect(resolveRouteStringRedirect('/@a', '/a/@a', '/@a')).toEqual('/a/@a')
+    expect(resolveRouteStringRedirect('/@a/@b', '/a/@a/@b', '/@b/1')).toEqual('/a/@b/1')
   })
   it('handles invalid redirects', () => {
     expectErr(

vike/shared/route/resolveRedirects.ts

@@ -6,6 +6,7 @@ export { resolveRouteStringRedirect }
 import { assertIsNotBrowser } from '../../utils/assertIsNotBrowser.js'
 import { isUriWithProtocol } from '../../utils/parseUrl-extras.js'
 import { assert, assertUsage } from '../utils.js'
+import { resolveUrlPathname } from './resolveUrlPathname.js'
 import { assertRouteString, resolveRouteString } from './resolveRouteString.js'
 import pc from '@brillout/picocolors'
 assertIsNotBrowser() // Don't bloat the client
@@ -37,13 +38,7 @@ function resolveRouteStringRedirect(urlSource: string, urlTarget: string, urlPat
   assertParams(urlSource, urlTarget)
   const match = resolveRouteString(urlSource, urlPathname)
   if (!match) return null
-  let urlResolved = urlTarget
-  Object.entries(match.routeParams).forEach(([key, val]) => {
-    if (key !== '*') {
-      key = `@${key}`
-    }
-    urlResolved = urlResolved.replaceAll(key, val)
-  })
+  const urlResolved = resolveUrlPathname(urlTarget, match.routeParams)
   if (urlResolved === urlPathname) return null
   assert(urlResolved.startsWith('/') || isUriWithProtocol(urlResolved))
   return urlResolved

vike/shared/route/resolveUrlPathname.spec.ts

@@ -0,0 +1,22 @@
+import { expect, describe, it } from 'vitest'
+import { resolveUrlPathname } from './resolveUrlPathname'
+
+describe('resolveUrlPathname', () => {
+  it('basics', () => {
+    expect(resolveUrlPathname('/', {})).toEqual('/')
+    expect(resolveUrlPathname('/a', {})).toEqual('/a')
+    expect(resolveUrlPathname('/@a', { a: '1' })).toEqual('/1')
+  })
+
+  it('edge cases', () => {
+    expect(resolveUrlPathname('/@a', { a: 'a' })).toEqual('/a')
+    expect(resolveUrlPathname('/@a/@b', { a: '@b', b: '1' })).toEqual('/@b/1')
+    expect(resolveUrlPathname('/@a/*', { a: '*', '*': '1' })).toEqual('/*/1')
+    expect(resolveUrlPathname('/*', { '*': '*' })).toEqual('/*')
+  })
+
+  // Not implemented yet
+  // it('multiple globs', () => {
+  //   expect(resolveUrlPathname('/*/*', { '*1': '*2', '*2': '*' })).toEqual('/*2/*')
+  // })
+})

vike/shared/route/resolveUrlPathname.ts

@@ -0,0 +1,56 @@
+export { resolveUrlPathname }
+
+import { assertIsNotBrowser } from '../../utils/assertIsNotBrowser.js'
+import { assert, assertUsage } from '../utils.js'
+assertIsNotBrowser() // Don't bloat the client
+
+type Part =
+  | {
+      type: 'ROUTE_STRING'
+      val: string
+    }
+  | {
+      type: 'URL'
+      val: string
+    }
+/** Given a `routeString` and `routeParams`, resolve `urlPathname`.
+ *
+ * Basically, the correct implementation of following:
+ * ```js
+ * let urlPathname = routeString
+ * Object.entries(routeParams).forEach(([key, val]) => {
+ *   urlPathname = urlPathname.replaceAll(key, val)
+ * })
+ * ```
+ */
+function resolveUrlPathname(routeString: string, routeParams: Record<string, string>): string {
+  let parts: Part[] = [{ val: routeString, type: 'ROUTE_STRING' }]
+
+  Object.entries(routeParams).forEach(([key, val]) => {
+    if (key.startsWith('*')) {
+      assert(key === '*' || /\d+/.test(key.slice(1)))
+      assertUsage(key === '*', "Resolving URL with multiple globs isn't implemented yet")
+    } else {
+      key = `@${key}`
+    }
+    parts = parts
+      .map((part) => {
+        if (part.type === 'URL') {
+          return part
+        } else {
+          return part.val
+            .split(key)
+            .map((rest, i) => {
+              const partURL = { val, type: 'URL' as const }
+              const partRouteString = { val: rest, type: 'ROUTE_STRING' as const }
+              return i === 0 ? [partRouteString] : [partURL, partRouteString]
+            })
+            .flat()
+        }
+      })
+      .flat()
+  })
+
+  const urlPathname = parts.map((p) => p.val).join('')
+  return urlPathname
+}