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

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

@@ -5314,6 +5314,60 @@ 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 OptRouteComponent = () => {
+      const params = useParams({ strict: false })
+      return <pre>{JSON.stringify(params)}</pre>
+    }
+    const optRoute = createRoute({
+      getParentRoute: () => rootRoute,
+      path: '/{-$foo}/bar',
+      component: OptRouteComponent,
+    })
+
+    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', () => {

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/router-core/src/router.ts

@@ -1860,15 +1860,20 @@ 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.params === true ||
+        typeof dest.params === 'function' ||
+        ((dest.from || !isAbsoluteTo) &&
+          dest.params !== false &&
+          dest.params !== null)
+      const baseParams = inheritParams ? fromParams : Object.create(null)
+      const nextParams = inheritParams
+        ? dest.params === true
+          ? baseParams
+          : Object.assign(baseParams, functionalUpdate(dest.params, 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({

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

@@ -5286,6 +5286,63 @@ 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' })
+
+    await fireEvent.click(fooBarLink)
+    await waitFor(() =>
+      expect(fooBarLink).toHaveAttribute('aria-current', 'page'),
+    )
+    expect(barLink).not.toHaveAttribute('aria-current', 'page')
+
+    await fireEvent.click(indexLink)
+    await waitFor(() =>
+      expect(indexLink).toHaveAttribute('aria-current', 'page'),
+    )
+
+    await fireEvent.click(barLink)
+    await waitFor(() => expect(barLink).toHaveAttribute('aria-current', 'page'))
+    expect(fooBarLink).not.toHaveAttribute('aria-current', 'page')
+  })
 })
 
 describe('createLink', () => {