Skip to content

Commit 99e3025

Browse files
SheraffSheraff
authored
fix(react-router): keep relative link active when changing inherited param (#6689)
1 parent d5e5fbe commit 99e3025

File tree

5 files changed

+368
-11
lines changed

5 files changed

+368
-11
lines changed

‎packages/react-router/src/link.tsx‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -376,10 +376,17 @@ export function useLinkProps<
376376
// eslint-disable-next-line react-hooks/rules-of-hooks
377377
const isHydrated = useHydrated()
378378

379-
// subscribe to search params to re-build location if it changes
379+
// subscribe to path/search/hash/params to re-build location when they change
380380
// eslint-disable-next-line react-hooks/rules-of-hooks
381-
const currentSearch = useRouterState({
382-
select: (s) => s.location.search,
381+
const currentLocationState = useRouterState({
382+
select: (s) => {
383+
const leaf = s.matches[s.matches.length - 1]
384+
return {
385+
search: leaf?.search,
386+
hash: s.location.hash,
387+
path: leaf?.pathname, // path + params
388+
}
389+
},
383390
structuralSharing: true as any,
384391
})
385392

@@ -393,7 +400,7 @@ export function useLinkProps<
393400
// eslint-disable-next-line react-hooks/exhaustive-deps
394401
[
395402
router,
396-
currentSearch,
403+
currentLocationState,
397404
from,
398405
options._fromLocation,
399406
options.hash,
@@ -556,11 +563,13 @@ export function useLinkProps<
556563

557564
// eslint-disable-next-line react-hooks/rules-of-hooks
558565
const doPreload = React.useCallback(() => {
559-
router.preloadRoute({ ..._options } as any).catch((err) => {
560-
console.warn(err)
561-
console.warn(preloadWarning)
562-
})
563-
}, [router, _options])
566+
router
567+
.preloadRoute({ ..._options, _builtLocation: next } as any)
568+
.catch((err) => {
569+
console.warn(err)
570+
console.warn(preloadWarning)
571+
})
572+
}, [router, _options, next])
564573

565574
// eslint-disable-next-line react-hooks/rules-of-hooks
566575
const preloadViewportIoCallback = React.useCallback(

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

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,6 +1521,120 @@ describe('Link', () => {
15211521
expect(paramText).toBeInTheDocument()
15221522
})
15231523

1524+
test('keeps a relative link active when changing inherited params (issue #5655)', async () => {
1525+
const rootRoute = createRootRoute()
1526+
1527+
const PostRouteComponent = () => {
1528+
const { postId } = useParams({ strict: false })
1529+
1530+
return (
1531+
<>
1532+
<Link
1533+
data-testid="step1-link"
1534+
from="/post/$postId"
1535+
to="step1"
1536+
activeProps={{ className: 'active' }}
1537+
>
1538+
Step 1
1539+
</Link>
1540+
<Link
1541+
data-testid="step2-link"
1542+
from="/post/$postId"
1543+
to="step2"
1544+
params={{ postId }}
1545+
activeProps={{ className: 'active' }}
1546+
>
1547+
Step 2
1548+
</Link>
1549+
<Outlet />
1550+
</>
1551+
)
1552+
}
1553+
1554+
const postRoute = createRoute({
1555+
getParentRoute: () => rootRoute,
1556+
path: '/post/$postId',
1557+
component: PostRouteComponent,
1558+
})
1559+
1560+
const Step1RouteComponent = () => {
1561+
const { postId } = useParams({ strict: false })
1562+
const otherPostId = postId === '1' ? '2' : '1'
1563+
1564+
return (
1565+
<>
1566+
<span>{`Post ${postId} step1`}</span>
1567+
<Link
1568+
data-testid="switch-post-link"
1569+
from="/post/$postId/step1"
1570+
to="."
1571+
params={{ postId: otherPostId }}
1572+
>{`Go to post ${otherPostId}`}</Link>
1573+
</>
1574+
)
1575+
}
1576+
const step1Route = createRoute({
1577+
getParentRoute: () => postRoute,
1578+
path: 'step1',
1579+
component: Step1RouteComponent,
1580+
})
1581+
1582+
const Step2RouteComponent = () => {
1583+
const { postId } = useParams({ strict: false })
1584+
const otherPostId = postId === '1' ? '2' : '1'
1585+
1586+
return (
1587+
<>
1588+
<span>{`Post ${postId} step2`}</span>
1589+
<Link
1590+
data-testid="switch-post-link"
1591+
from="/post/$postId/step2"
1592+
to="."
1593+
params={{ postId: otherPostId }}
1594+
>{`Go to post ${otherPostId}`}</Link>
1595+
</>
1596+
)
1597+
}
1598+
const step2Route = createRoute({
1599+
getParentRoute: () => postRoute,
1600+
path: 'step2',
1601+
component: Step2RouteComponent,
1602+
})
1603+
1604+
const router = createRouter({
1605+
routeTree: rootRoute.addChildren([
1606+
postRoute.addChildren([step1Route, step2Route]),
1607+
]),
1608+
history: createMemoryHistory({
1609+
initialEntries: ['/post/1/step1'],
1610+
}),
1611+
})
1612+
1613+
render(<RouterProvider router={router} />)
1614+
1615+
expect(await screen.findByText('Post 1 step1')).toBeInTheDocument()
1616+
expect(screen.getByTestId('step1-link')).toHaveClass('active')
1617+
1618+
await act(() => fireEvent.click(screen.getByTestId('switch-post-link')))
1619+
1620+
expect(await screen.findByText('Post 2 step1')).toBeInTheDocument()
1621+
expect(router.state.location.pathname).toBe('/post/2/step1')
1622+
// This is the bug from #5655: step1 should stay active but is not.
1623+
expect(screen.getByTestId('step1-link')).toHaveClass('active')
1624+
1625+
await act(() => fireEvent.click(screen.getByTestId('step2-link')))
1626+
1627+
expect(await screen.findByText('Post 2 step2')).toBeInTheDocument()
1628+
expect(router.state.location.pathname).toBe('/post/2/step2')
1629+
expect(screen.getByTestId('step2-link')).toHaveClass('active')
1630+
1631+
await act(() => fireEvent.click(screen.getByTestId('switch-post-link')))
1632+
1633+
expect(await screen.findByText('Post 1 step2')).toBeInTheDocument()
1634+
expect(router.state.location.pathname).toBe('/post/1/step2')
1635+
expect(screen.getByTestId('step2-link')).toHaveClass('active')
1636+
})
1637+
15241638
test('when navigating from /posts to ./$postId', async () => {
15251639
const rootRoute = createRootRoute()
15261640
const indexRoute = createRoute({

‎packages/router-core/src/router.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,13 @@ export type PreloadRouteFn<
661661
TTo,
662662
TMaskFrom,
663663
TMaskTo
664-
>,
664+
> & {
665+
/**
666+
* @internal
667+
* A **trusted** built location that can be used to redirect to.
668+
*/
669+
_builtLocation?: ParsedLocation
670+
},
665671
) => Promise<Array<AnyRouteMatch> | undefined>
666672

667673
export type MatchRouteFn<
@@ -2757,7 +2763,7 @@ export class RouterCore<
27572763
TDefaultStructuralSharingOption,
27582764
TRouterHistory
27592765
> = async (opts) => {
2760-
const next = this.buildLocation(opts as any)
2766+
const next = opts._builtLocation ?? this.buildLocation(opts as any)
27612767

27622768
let matches = this.matchRoutes(next, {
27632769
throwOnError: true,

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

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,6 +1546,118 @@ describe('Link', () => {
15461546
expect(paramText).toBeInTheDocument()
15471547
})
15481548

1549+
test('keeps a relative link active when changing inherited params (issue #5655)', async () => {
1550+
const rootRoute = createRootRoute()
1551+
1552+
const postRoute = createRoute({
1553+
getParentRoute: () => rootRoute,
1554+
path: '/post/$postId',
1555+
component: () => {
1556+
const params = useParams({ strict: false })
1557+
const postId = () => params().postId
1558+
1559+
return (
1560+
<>
1561+
<Link
1562+
data-testid="step1-link"
1563+
from="/post/$postId"
1564+
to="step1"
1565+
activeProps={{ class: 'active' }}
1566+
>
1567+
Step 1
1568+
</Link>
1569+
<Link
1570+
data-testid="step2-link"
1571+
from="/post/$postId"
1572+
to="step2"
1573+
params={{ postId: postId() }}
1574+
activeProps={{ class: 'active' }}
1575+
>
1576+
Step 2
1577+
</Link>
1578+
<Outlet />
1579+
</>
1580+
)
1581+
},
1582+
})
1583+
1584+
const step1Route = createRoute({
1585+
getParentRoute: () => postRoute,
1586+
path: 'step1',
1587+
component: () => {
1588+
const params = useParams({ strict: false })
1589+
const postId = () => params().postId
1590+
const otherPostId = () => (postId() === '1' ? '2' : '1')
1591+
1592+
return (
1593+
<>
1594+
<span>{`Post ${postId()} step1`}</span>
1595+
<Link
1596+
data-testid="switch-post-link"
1597+
from="/post/$postId/step1"
1598+
to="."
1599+
params={{ postId: otherPostId() }}
1600+
>{`Go to post ${otherPostId()}`}</Link>
1601+
</>
1602+
)
1603+
},
1604+
})
1605+
1606+
const step2Route = createRoute({
1607+
getParentRoute: () => postRoute,
1608+
path: 'step2',
1609+
component: () => {
1610+
const params = useParams({ strict: false })
1611+
const postId = () => params().postId
1612+
const otherPostId = () => (postId() === '1' ? '2' : '1')
1613+
1614+
return (
1615+
<>
1616+
<span>{`Post ${postId()} step2`}</span>
1617+
<Link
1618+
data-testid="switch-post-link"
1619+
from="/post/$postId/step2"
1620+
to="."
1621+
params={{ postId: otherPostId() }}
1622+
>{`Go to post ${otherPostId()}`}</Link>
1623+
</>
1624+
)
1625+
},
1626+
})
1627+
1628+
const router = createRouter({
1629+
routeTree: rootRoute.addChildren([
1630+
postRoute.addChildren([step1Route, step2Route]),
1631+
]),
1632+
history: createMemoryHistory({
1633+
initialEntries: ['/post/1/step1'],
1634+
}),
1635+
})
1636+
1637+
render(() => <RouterProvider router={router} />)
1638+
1639+
expect(await screen.findByText('Post 1 step1')).toBeInTheDocument()
1640+
expect(screen.getByTestId('step1-link')).toHaveClass('active')
1641+
1642+
fireEvent.click(screen.getByTestId('switch-post-link'))
1643+
1644+
expect(await screen.findByText('Post 2 step1')).toBeInTheDocument()
1645+
expect(router.state.location.pathname).toBe('/post/2/step1')
1646+
expect(screen.getByTestId('step1-link')).toHaveClass('active')
1647+
1648+
fireEvent.click(screen.getByTestId('step2-link'))
1649+
1650+
expect(await screen.findByText('Post 2 step2')).toBeInTheDocument()
1651+
expect(router.state.location.pathname).toBe('/post/2/step2')
1652+
expect(screen.getByTestId('step2-link')).toHaveClass('active')
1653+
1654+
fireEvent.click(screen.getByTestId('switch-post-link'))
1655+
1656+
expect(await screen.findByText('Post 1 step2')).toBeInTheDocument()
1657+
expect(router.state.location.pathname).toBe('/post/1/step2')
1658+
expect(screen.getByTestId('step2-link')).toHaveClass('active')
1659+
})
1660+
15491661
test('when navigating from /posts to ./$postId', async () => {
15501662
const rootRoute = createRootRoute()
15511663
const indexRoute = createRoute({

0 commit comments

Comments
 (0)