fix: linking with from prop and a param containing slash (#1799)

* Add test case for linking with from prop and a param containing slash * Remove .only from test * fix: encode pathname in match * refactor: move encoded params to interpolatePath --------- Co-authored-by: Jakob Norlin <[email protected]> Co-authored-by: chorobin <[email protected]>
TanStack · Jun 20, 2024 · 8149575 · 8149575
1 parent a095cd5
commit 8149575
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 13 deletions.
diff --git a/packages/react-router/src/path.ts b/packages/react-router/src/path.ts
@@ -205,21 +205,33 @@ export function interpolatePath({
   leaveParams,
 }: InterpolatePathOptions) {
   const interpolatedPathSegments = parsePathname(path)
+  const encodedParams: any = {}
+
+  for (const [key, value] of Object.entries(params)) {
+    const isValueString = typeof value === 'string'
+
+    if (['*', '_splat'].includes(key)) {
+      // the splat/catch-all routes shouldn't have the '/' encoded out
+      encodedParams[key] = isValueString ? encodeURI(value) : value
+    } else {
+      encodedParams[key] = isValueString ? encodeURIComponent(value) : value
+    }
+  }
 
   return joinPaths(
     interpolatedPathSegments.map((segment) => {
       if (segment.type === 'wildcard') {
-        const value = params._splat
+        const value = encodedParams._splat
         if (leaveWildcards) return `${segment.value}${value ?? ''}`
         return value
       }
 
       if (segment.type === 'param') {
         if (leaveParams) {
-          const value = params[segment.value]
+          const value = encodedParams[segment.value]
           return `${segment.value}${value ?? ''}`
         }
-        return params![segment.value.substring(1)] ?? 'undefined'
+        return encodedParams![segment.value.substring(1)] ?? 'undefined'
       }
 
       return segment.value

diff --git a/packages/react-router/src/router.ts b/packages/react-router/src/router.ts
@@ -1170,16 +1170,6 @@ export class Router<
           })
       }
 
-      // encode all path params so the generated href is valid and stable
-      Object.keys(nextParams).forEach((key) => {
-        if (['*', '_splat'].includes(key)) {
-          // the splat/catch-all routes shouldn't have the '/' encoded out
-          nextParams[key] = encodeURI(nextParams[key])
-        } else {
-          nextParams[key] = encodeURIComponent(nextParams[key])
-        }
-      })
-
       pathname = interpolatePath({
         path: pathname,
         params: nextParams ?? {},

diff --git a/packages/react-router/tests/link.test.tsx b/packages/react-router/tests/link.test.tsx
@@ -3030,6 +3030,54 @@ describe('Link', () => {
 
     expect(ErrorComponent).not.toHaveBeenCalled()
   })
+
+  test('when linking to self with from prop set and param containing a slash', async () => {
+    const ErrorComponent = vi.fn(() => <h1>Something went wrong!</h1>)
+
+    const rootRoute = createRootRoute({
+      errorComponent: ErrorComponent,
+    })
+
+    const indexRoute = createRoute({
+      getParentRoute: () => rootRoute,
+      path: '/',
+      component: () => (
+        <Link to="/$postId" params={{ postId: 'id/with-slash' }}>
+          Go to post
+        </Link>
+      ),
+    })
+
+    const postRoute = createRoute({
+      getParentRoute: () => rootRoute,
+      path: '/$postId',
+      component: () => (
+        <Link from="/$postId" to="/$postId">
+          Link to self with from prop set
+        </Link>
+      ),
+    })
+
+    const routeTree = rootRoute.addChildren([indexRoute, postRoute])
+    const router = createRouter({ routeTree })
+
+    render(<RouterProvider router={router} />)
+
+    const postLink = await screen.findByRole('link', {
+      name: 'Go to post',
+    })
+
+    expect(postLink).toHaveAttribute('href', '/id%2Fwith-slash')
+
+    fireEvent.click(postLink)
+
+    const selfLink = await screen.findByRole('link', {
+      name: 'Link to self with from prop set',
+    })
+
+    expect(selfLink).toBeInTheDocument()
+    expect(ErrorComponent).not.toHaveBeenCalled()
+  })
 })
 
 describe('createLink', () => {