diff --git a/packages/react-router/src/link.tsx b/packages/react-router/src/link.tsx index 60088cd1cb1..df431211e0f 100644 --- a/packages/react-router/src/link.tsx +++ b/packages/react-router/src/link.tsx @@ -376,10 +376,17 @@ export function useLinkProps< // eslint-disable-next-line react-hooks/rules-of-hooks const isHydrated = useHydrated() - // subscribe to search params to re-build location if it changes + // subscribe to path/search/hash/params to re-build location when they change // eslint-disable-next-line react-hooks/rules-of-hooks - const currentSearch = useRouterState({ - select: (s) => s.location.search, + const currentLocationState = useRouterState({ + select: (s) => { + const leaf = s.matches[s.matches.length - 1] + return { + search: leaf?.search, + hash: s.location.hash, + path: leaf?.pathname, // path + params + } + }, structuralSharing: true as any, }) @@ -393,7 +400,7 @@ export function useLinkProps< // eslint-disable-next-line react-hooks/exhaustive-deps [ router, - currentSearch, + currentLocationState, from, options._fromLocation, options.hash, @@ -556,11 +563,13 @@ export function useLinkProps< // eslint-disable-next-line react-hooks/rules-of-hooks const doPreload = React.useCallback(() => { - router.preloadRoute({ ..._options } as any).catch((err) => { - console.warn(err) - console.warn(preloadWarning) - }) - }, [router, _options]) + router + .preloadRoute({ ..._options, _builtLocation: next } as any) + .catch((err) => { + console.warn(err) + console.warn(preloadWarning) + }) + }, [router, _options, next]) // eslint-disable-next-line react-hooks/rules-of-hooks const preloadViewportIoCallback = React.useCallback( diff --git a/packages/react-router/tests/link.test.tsx b/packages/react-router/tests/link.test.tsx index da7078f785a..87c61e17385 100644 --- a/packages/react-router/tests/link.test.tsx +++ b/packages/react-router/tests/link.test.tsx @@ -1521,6 +1521,120 @@ describe('Link', () => { expect(paramText).toBeInTheDocument() }) + test('keeps a relative link active when changing inherited params (issue #5655)', async () => { + const rootRoute = createRootRoute() + + const PostRouteComponent = () => { + const { postId } = useParams({ strict: false }) + + return ( + <> + + Step 1 + + + Step 2 + + + + ) + } + + const postRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/post/$postId', + component: PostRouteComponent, + }) + + const Step1RouteComponent = () => { + const { postId } = useParams({ strict: false }) + const otherPostId = postId === '1' ? '2' : '1' + + return ( + <> + {`Post ${postId} step1`} + {`Go to post ${otherPostId}`} + + ) + } + const step1Route = createRoute({ + getParentRoute: () => postRoute, + path: 'step1', + component: Step1RouteComponent, + }) + + const Step2RouteComponent = () => { + const { postId } = useParams({ strict: false }) + const otherPostId = postId === '1' ? '2' : '1' + + return ( + <> + {`Post ${postId} step2`} + {`Go to post ${otherPostId}`} + + ) + } + const step2Route = createRoute({ + getParentRoute: () => postRoute, + path: 'step2', + component: Step2RouteComponent, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([ + postRoute.addChildren([step1Route, step2Route]), + ]), + history: createMemoryHistory({ + initialEntries: ['/post/1/step1'], + }), + }) + + render() + + expect(await screen.findByText('Post 1 step1')).toBeInTheDocument() + expect(screen.getByTestId('step1-link')).toHaveClass('active') + + await act(() => fireEvent.click(screen.getByTestId('switch-post-link'))) + + expect(await screen.findByText('Post 2 step1')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/2/step1') + // This is the bug from #5655: step1 should stay active but is not. + expect(screen.getByTestId('step1-link')).toHaveClass('active') + + await act(() => fireEvent.click(screen.getByTestId('step2-link'))) + + expect(await screen.findByText('Post 2 step2')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/2/step2') + expect(screen.getByTestId('step2-link')).toHaveClass('active') + + await act(() => fireEvent.click(screen.getByTestId('switch-post-link'))) + + expect(await screen.findByText('Post 1 step2')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/1/step2') + expect(screen.getByTestId('step2-link')).toHaveClass('active') + }) + test('when navigating from /posts to ./$postId', async () => { const rootRoute = createRootRoute() const indexRoute = createRoute({ diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index e59c87bb409..5ff3a6e93a3 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -661,7 +661,13 @@ export type PreloadRouteFn< TTo, TMaskFrom, TMaskTo - >, + > & { + /** + * @internal + * A **trusted** built location that can be used to redirect to. + */ + _builtLocation?: ParsedLocation + }, ) => Promise | undefined> export type MatchRouteFn< @@ -2757,7 +2763,7 @@ export class RouterCore< TDefaultStructuralSharingOption, TRouterHistory > = async (opts) => { - const next = this.buildLocation(opts as any) + const next = opts._builtLocation ?? this.buildLocation(opts as any) let matches = this.matchRoutes(next, { throwOnError: true, diff --git a/packages/solid-router/tests/link.test.tsx b/packages/solid-router/tests/link.test.tsx index 5c7fc93d0f7..d5c528c0add 100644 --- a/packages/solid-router/tests/link.test.tsx +++ b/packages/solid-router/tests/link.test.tsx @@ -1546,6 +1546,118 @@ describe('Link', () => { expect(paramText).toBeInTheDocument() }) + test('keeps a relative link active when changing inherited params (issue #5655)', async () => { + const rootRoute = createRootRoute() + + const postRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/post/$postId', + component: () => { + const params = useParams({ strict: false }) + const postId = () => params().postId + + return ( + <> + + Step 1 + + + Step 2 + + + + ) + }, + }) + + const step1Route = createRoute({ + getParentRoute: () => postRoute, + path: 'step1', + component: () => { + const params = useParams({ strict: false }) + const postId = () => params().postId + const otherPostId = () => (postId() === '1' ? '2' : '1') + + return ( + <> + {`Post ${postId()} step1`} + {`Go to post ${otherPostId()}`} + + ) + }, + }) + + const step2Route = createRoute({ + getParentRoute: () => postRoute, + path: 'step2', + component: () => { + const params = useParams({ strict: false }) + const postId = () => params().postId + const otherPostId = () => (postId() === '1' ? '2' : '1') + + return ( + <> + {`Post ${postId()} step2`} + {`Go to post ${otherPostId()}`} + + ) + }, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([ + postRoute.addChildren([step1Route, step2Route]), + ]), + history: createMemoryHistory({ + initialEntries: ['/post/1/step1'], + }), + }) + + render(() => ) + + expect(await screen.findByText('Post 1 step1')).toBeInTheDocument() + expect(screen.getByTestId('step1-link')).toHaveClass('active') + + fireEvent.click(screen.getByTestId('switch-post-link')) + + expect(await screen.findByText('Post 2 step1')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/2/step1') + expect(screen.getByTestId('step1-link')).toHaveClass('active') + + fireEvent.click(screen.getByTestId('step2-link')) + + expect(await screen.findByText('Post 2 step2')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/2/step2') + expect(screen.getByTestId('step2-link')).toHaveClass('active') + + fireEvent.click(screen.getByTestId('switch-post-link')) + + expect(await screen.findByText('Post 1 step2')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/1/step2') + expect(screen.getByTestId('step2-link')).toHaveClass('active') + }) + test('when navigating from /posts to ./$postId', async () => { const rootRoute = createRootRoute() const indexRoute = createRoute({ diff --git a/packages/vue-router/tests/link.test.tsx b/packages/vue-router/tests/link.test.tsx index 0165bbc0928..3827adc6088 100644 --- a/packages/vue-router/tests/link.test.tsx +++ b/packages/vue-router/tests/link.test.tsx @@ -1547,6 +1547,122 @@ describe('Link', () => { expect(paramText).toBeInTheDocument() }) + test('keeps a relative link active when changing inherited params (issue #5655)', async () => { + const rootRoute = createRootRoute() + + const postRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/post/$postId', + component: () => { + const params = useParams({ strict: false }) + const postId = Vue.computed(() => params.value.postId) + + return ( + <> + + Step 1 + + + Step 2 + + + + ) + }, + }) + + const step1Route = createRoute({ + getParentRoute: () => postRoute, + path: 'step1', + component: () => { + const params = useParams({ strict: false }) + const postId = Vue.computed(() => params.value.postId) + const otherPostId = Vue.computed(() => + postId.value === '1' ? '2' : '1', + ) + + return ( + <> + {`Post ${postId.value} step1`} + {`Go to post ${otherPostId.value}`} + + ) + }, + }) + + const step2Route = createRoute({ + getParentRoute: () => postRoute, + path: 'step2', + component: () => { + const params = useParams({ strict: false }) + const postId = Vue.computed(() => params.value.postId) + const otherPostId = Vue.computed(() => + postId.value === '1' ? '2' : '1', + ) + + return ( + <> + {`Post ${postId.value} step2`} + {`Go to post ${otherPostId.value}`} + + ) + }, + }) + + const router = createRouter({ + routeTree: rootRoute.addChildren([ + postRoute.addChildren([step1Route, step2Route]), + ]), + history: createMemoryHistory({ + initialEntries: ['/post/1/step1'], + }), + }) + + render() + + expect(await screen.findByText('Post 1 step1')).toBeInTheDocument() + expect(screen.getByTestId('step1-link')).toHaveClass('active') + + fireEvent.click(screen.getByTestId('switch-post-link')) + + expect(await screen.findByText('Post 2 step1')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/2/step1') + expect(screen.getByTestId('step1-link')).toHaveClass('active') + + fireEvent.click(screen.getByTestId('step2-link')) + + expect(await screen.findByText('Post 2 step2')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/2/step2') + expect(screen.getByTestId('step2-link')).toHaveClass('active') + + fireEvent.click(screen.getByTestId('switch-post-link')) + + expect(await screen.findByText('Post 1 step2')).toBeInTheDocument() + expect(router.state.location.pathname).toBe('/post/1/step2') + expect(screen.getByTestId('step2-link')).toHaveClass('active') + }) + test('when navigating from /posts to ./$postId', async () => { const rootRoute = createRootRoute() const indexRoute = createRoute({