fix(react-router): Link isActive on competing optional segment routes

packages/react-router/tests/link.test.tsx

@@ -5314,6 +5314,59 @@ describe('Link', () => {
       await runTest({ expectedPreload: false, testIdToHover: 'link-2' })
     })
   })
+
+  test('edge-case: competing optional segment links', async () => {
+    const rootRoute = createRootRoute({
+      component: () => {
+        return (
+          <>
+            <Link to="/">To index</Link>
+            <Link to="/{-$foo}/bar" params={{ foo: 'foo' }}>
+              To /foo/bar
+            </Link>
+            <Link to="/{-$foo}/bar">To /bar</Link>
+            <Outlet />
+          </>
+        )
+      },
+    })
+    const indexRoute = createRoute({
+      getParentRoute: () => rootRoute,
+      path: '/',
+      component: () => <p>index</p>,
+    })
+    const optRoute = createRoute({
+      getParentRoute: () => rootRoute,
+      path: '/{-$foo}/bar',
+      component: () => {
+        const params = useParams({ strict: false })
+        return <pre>{JSON.stringify(params)}</pre>
+      },
+    })
+
+    const router = createRouter({
+      routeTree: rootRoute.addChildren([indexRoute, optRoute]),
+      history,
+    })
+
+    render(<RouterProvider router={router} />)
+    expect(await screen.findByText('index')).toBeInTheDocument()
+
+    const indexLink = await screen.findByRole('link', { name: 'To index' })
+    const fooBarLink = await screen.findByRole('link', { name: 'To /foo/bar' })
+    const barLink = await screen.findByRole('link', { name: 'To /bar' })
+
+    fireEvent.click(fooBarLink)
+    expect(fooBarLink).toHaveAttribute('aria-current', 'page')
+    expect(barLink).not.toHaveAttribute('aria-current', 'page')
+
+    fireEvent.click(indexLink)
+    expect(indexLink).toHaveAttribute('aria-current', 'page')
+
+    fireEvent.click(barLink)
+    expect(fooBarLink).not.toHaveAttribute('aria-current', 'page')
+    expect(barLink).toHaveAttribute('aria-current', 'page')
+  })
 })
 
 describe('createLink', () => {
@@ -5952,7 +6005,9 @@ describe.each([{ basepath: '' }, { basepath: '/basepath' }])(
           return (
             <>
               <h1>B Route</h1>
-              <Link to="..">Link to Parent</Link>
+              <Link to=".." params={true}>
+                Link to Parent
+              </Link>
             </>
           )
         },
@@ -5990,7 +6045,7 @@ describe.each([{ basepath: '' }, { basepath: '/basepath' }])(
               <Link from={paramARoute.fullPath} to="..">
                 Link to .. from /param/foo/a
               </Link>
-              <Link to=".." data-testid={'link-to-previous'}>
+              <Link to=".." params={true} data-testid={'link-to-previous'}>
                 Link to .. from current active route
               </Link>
               <Outlet />
@@ -6006,7 +6061,9 @@ describe.each([{ basepath: '' }, { basepath: '/basepath' }])(
           return (
             <>
               <h1>Param B Route</h1>
-              <Link to="..">Link to Parent</Link>
+              <Link to=".." params={true}>
+                Link to Parent
+              </Link>
               <Link to="." params={{ param: 'bar' }}>
                 Link to . with param:bar
               </Link>
@@ -6833,10 +6890,10 @@ describe('relative links to from route', () => {
         return (
           <>
             <h1 data-testid="post-detail-index-heading">Post Detail</h1>
-            <Link data-testid="post-info-link" to="info">
+            <Link data-testid="post-info-link" to="info" params={true}>
               To post info
             </Link>
-            <Link data-testid="post-notes-link" to="notes">
+            <Link data-testid="post-notes-link" to="notes" params={true}>
               To post notes
             </Link>
             <Link

packages/react-router/tests/navigate.test.tsx

@@ -195,6 +195,7 @@ describe('router.navigate navigation using multiple path params - object syntax
     expect(router.state.location.pathname).toBe('/p/router/v1/react')
 
     await router.navigate({
+      from: '/p/$projectId/$version/$framework',
       to: '/p/$projectId/$version/$framework',
       params: { projectId: 'query' },
     })
@@ -213,6 +214,7 @@ describe('router.navigate navigation using multiple path params - object syntax
     expect(router.state.location.pathname).toBe('/p/router/v1/react')
 
     await router.navigate({
+      from: '/p/$projectId/$version/$framework',
       params: { projectId: 'query' },
     } as any)
     await router.invalidate()
@@ -230,6 +232,7 @@ describe('router.navigate navigation using multiple path params - object syntax
     expect(router.state.location.pathname).toBe('/p/router/v1/react')
 
     await router.navigate({
+      from: '/p/$projectId/$version/$framework',
       to: '/p/$projectId/$version/$framework',
       params: { version: 'v3' },
     })
@@ -248,6 +251,7 @@ describe('router.navigate navigation using multiple path params - object syntax
     expect(router.state.location.pathname).toBe('/p/router/v1/react')
 
     await router.navigate({
+      from: '/p/$projectId/$version/$framework',
       params: { version: 'v3' },
     } as any)
     await router.invalidate()
@@ -265,6 +269,7 @@ describe('router.navigate navigation using multiple path params - object syntax
     expect(router.state.location.pathname).toBe('/p/router/v1/react')
 
     await router.navigate({
+      from: '/p/$projectId/$version/$framework',
       to: '/p/$projectId/$version/$framework',
       params: { framework: 'vue' },
     })
@@ -283,6 +288,7 @@ describe('router.navigate navigation using multiple path params - object syntax
     expect(router.state.location.pathname).toBe('/p/router/v1/react')
 
     await router.navigate({
+      from: '/p/$projectId/$version/$framework',
       params: { framework: 'vue' },
     } as any)
     await router.invalidate()
@@ -778,7 +784,7 @@ describe('router.navigate navigation using optional path parameters - object syn
     expect(router.state.location.pathname).toBe('/p/router/vue')
   })
 
-  it('should carry over optional parameters from current route when using empty params', async () => {
+  it('should carry over optional parameters from current route when params is true', async () => {
     const { router } = createOptionalParamTestRouter(
       createMemoryHistory({ initialEntries: ['/posts/tech'] }),
     )
@@ -798,7 +804,7 @@ describe('router.navigate navigation using optional path parameters - object syn
     // Navigate back to posts - should carry over 'news' from current params
     await router.navigate({
       to: '/posts/{-$category}',
-      params: {},
+      params: true,
     })
     await router.invalidate()
 
@@ -1039,7 +1045,7 @@ describe('router.navigate navigation using optional path parameters - parameter
     // Navigate to articles without specifying category
     await router.navigate({
       to: '/articles/{-$category}',
-      params: {},
+      params: true,
     })
     await router.invalidate()
 
@@ -1067,7 +1073,7 @@ describe('router.navigate navigation using optional path parameters - parameter
     // Navigate back to posts without explicit category removal
     await router.navigate({
       to: '/posts/{-$category}',
-      params: {},
+      params: true,
     })
     await router.invalidate()
 

packages/react-router/tests/useNavigate.test.tsx

@@ -1659,6 +1659,7 @@ test.each([true, false])(
         (open: boolean) => {
           navigate({
             to: '.',
+            params: true,
             search: (prev: {}) => ({
               ...prev,
               [`_${name}`]: open ? true : undefined,
@@ -2352,7 +2353,7 @@ describe.each([{ basepath: '' }, { basepath: '/basepath' }])(
                 Link to .. from /param/foo/a
               </button>
               <button
-                onClick={() => navigate({ to: '..' })}
+                onClick={() => navigate({ to: '..', params: true })}
                 data-testid={'link-to-previous'}
               >
                 Link to .. from current active route
@@ -2371,7 +2372,7 @@ describe.each([{ basepath: '' }, { basepath: '/basepath' }])(
           return (
             <>
               <h1>Param B Route</h1>
-              <button onClick={() => navigate({ to: '..' })}>
+              <button onClick={() => navigate({ to: '..', params: true })}>
                 Link to Parent
               </button>
               <button

packages/router-core/src/router.ts

@@ -1860,15 +1860,19 @@ export class RouterCore<
         : sourcePath
 
       // Resolve the next params
-      const nextParams =
-        dest.params === false || dest.params === null
-          ? Object.create(null)
-          : (dest.params ?? true) === true
-            ? fromParams
-            : Object.assign(
-                fromParams,
-                functionalUpdate(dest.params as any, fromParams),
-              )
+      const inheritParams =
+        dest.from || dest.params === true || typeof dest.params === 'function'
+      const baseParams = inheritParams ? fromParams : Object.create(null)
+      const nextParams = inheritParams
+        ? dest.params === true
+          ? baseParams
+          : Object.assign(
+              baseParams,
+              functionalUpdate(dest.params as any, fromParams),
+            )
+        : dest.params
+          ? Object.assign(baseParams, dest.params)
+          : baseParams
 
       const destRoute = this.routesByPath[
         trimPathRight(nextTo) as keyof typeof this.routesByPath

packages/router-core/tests/build-location.test.ts

@@ -1036,6 +1036,141 @@ describe('buildLocation - params edge cases', () => {
     expect(location.pathname).toBe('/users/456')
   })
 
+  test('omitted params should not inherit current params without from', async () => {
+    const rootRoute = new BaseRootRoute({})
+    const orgRoute = new BaseRoute({
+      getParentRoute: () => rootRoute,
+      path: '/orgs/$orgId',
+    })
+    const userRoute = new BaseRoute({
+      getParentRoute: () => orgRoute,
+      path: '/users/$userId',
+    })
+
+    const routeTree = rootRoute.addChildren([orgRoute.addChildren([userRoute])])
+
+    const router = createTestRouter({
+      routeTree,
+      history: createMemoryHistory({ initialEntries: ['/orgs/abc/users/123'] }),
+    })
+
+    await router.load()
+
+    const location = router.buildLocation({
+      to: '/orgs/$orgId/users/$userId',
+    })
+
+    expect(location.pathname).toBe('/orgs/undefined/users/undefined')
+  })
+
+  test('omitted params should not inherit current optional params without from', async () => {
+    const rootRoute = new BaseRootRoute({})
+    const optRoute = new BaseRoute({
+      getParentRoute: () => rootRoute,
+      path: '/{-$foo}/bar',
+    })
+
+    const routeTree = rootRoute.addChildren([optRoute])
+
+    const router = createTestRouter({
+      routeTree,
+      history: createMemoryHistory({ initialEntries: ['/foo/bar'] }),
+    })
+
+    await router.load()
+
+    const location = router.buildLocation({
+      to: '/{-$foo}/bar',
+    })
+
+    expect(location.pathname).toBe('/bar')
+  })
+
+  test('params as object should not merge current params without from', async () => {
+    const rootRoute = new BaseRootRoute({})
+    const orgRoute = new BaseRoute({
+      getParentRoute: () => rootRoute,
+      path: '/orgs/$orgId',
+    })
+    const userRoute = new BaseRoute({
+      getParentRoute: () => orgRoute,
+      path: '/users/$userId',
+    })
+
+    const routeTree = rootRoute.addChildren([orgRoute.addChildren([userRoute])])
+
+    const router = createTestRouter({
+      routeTree,
+      history: createMemoryHistory({ initialEntries: ['/orgs/abc/users/123'] }),
+    })
+
+    await router.load()
+
+    const location = router.buildLocation({
+      to: '/orgs/$orgId/users/$userId',
+      params: { userId: '456' },
+    })
+
+    expect(location.pathname).toBe('/orgs/undefined/users/456')
+  })
+
+  test('omitted params should inherit params with explicit from', async () => {
+    const rootRoute = new BaseRootRoute({})
+    const orgRoute = new BaseRoute({
+      getParentRoute: () => rootRoute,
+      path: '/orgs/$orgId',
+    })
+    const userRoute = new BaseRoute({
+      getParentRoute: () => orgRoute,
+      path: '/users/$userId',
+    })
+
+    const routeTree = rootRoute.addChildren([orgRoute.addChildren([userRoute])])
+
+    const router = createTestRouter({
+      routeTree,
+      history: createMemoryHistory({ initialEntries: ['/orgs/abc/users/123'] }),
+    })
+
+    await router.load()
+
+    const location = router.buildLocation({
+      from: '/orgs/$orgId/users/$userId',
+      to: '/orgs/$orgId/users/$userId',
+    })
+
+    expect(location.pathname).toBe('/orgs/abc/users/123')
+  })
+
+  test('params as object should merge params with explicit from', async () => {
+    const rootRoute = new BaseRootRoute({})
+    const orgRoute = new BaseRoute({
+      getParentRoute: () => rootRoute,
+      path: '/orgs/$orgId',
+    })
+    const userRoute = new BaseRoute({
+      getParentRoute: () => orgRoute,
+      path: '/users/$userId',
+    })
+
+    const routeTree = rootRoute.addChildren([orgRoute.addChildren([userRoute])])
+
+    const router = createTestRouter({
+      routeTree,
+      history: createMemoryHistory({ initialEntries: ['/orgs/abc/users/123'] }),
+    })
+
+    await router.load()
+
+    const location = router.buildLocation({
+      from: '/orgs/$orgId/users/$userId',
+      to: '/orgs/$orgId/users/$userId',
+      params: { userId: '456' },
+    })
+
+    expect(location.pathname).toBe('/orgs/abc/users/456')
+  })
+
   test('params as object should merge with current params', async () => {
     const rootRoute = new BaseRootRoute({})
     const orgRoute = new BaseRoute({