diff --git a/packages/react-router/src/useBlocker.tsx b/packages/react-router/src/useBlocker.tsx index 68612c97cb3..65636510c9d 100644 --- a/packages/react-router/src/useBlocker.tsx +++ b/packages/react-router/src/useBlocker.tsx @@ -179,22 +179,24 @@ export function useBlocker( location: HistoryLocation, ): AnyShouldBlockFnLocation { const parsedLocation = router.parseLocation(location) - const matchedRoutes = router.getMatchedRoutes(parsedLocation.pathname) - if (matchedRoutes.foundRoute === undefined) { + const [, rawParams, foundRoute] = router.getMatchedRoutes( + parsedLocation.pathname, + ) + if (foundRoute === undefined) { return { routeId: '__notFound__', fullPath: parsedLocation.pathname, pathname: parsedLocation.pathname, - params: matchedRoutes.routeParams, + params: rawParams, search: router.options.parseSearch(location.search), } } return { - routeId: matchedRoutes.foundRoute.id, - fullPath: matchedRoutes.foundRoute.fullPath, + routeId: foundRoute.id, + fullPath: foundRoute.fullPath, pathname: parsedLocation.pathname, - params: matchedRoutes.routeParams, + params: rawParams, search: router.options.parseSearch(location.search), } } diff --git a/packages/react-router/tests/issue-7964-param-parsing-loader.test.tsx b/packages/react-router/tests/issue-7964-param-parsing-loader.test.tsx new file mode 100644 index 00000000000..54f6c19c47f --- /dev/null +++ b/packages/react-router/tests/issue-7964-param-parsing-loader.test.tsx @@ -0,0 +1,107 @@ +import { act, cleanup, render, screen, waitFor } from '@testing-library/react' +import { afterEach, expect, test } from 'vitest' +import { + Outlet, + RouterProvider, + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, +} from '../src' + +afterEach(() => { + cleanup() +}) + +// https://github.com/TanStack/router/issues/7964 +test('#7964: a child loader receives fresh structured params after revisiting its route', async () => { + const rootRoute = createRootRoute({ + component: Outlet, + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const parsedParamRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/$parsedParam', + params: { + parse: ({ parsedParam }) => { + const [recordName, revisionNumber] = parsedParam.split('-') + return { + parsedParam: { + recordName: recordName!, + revisionNumber: Number(revisionNumber), + }, + } + }, + stringify: ({ parsedParam }) => ({ + parsedParam: `${parsedParam.recordName}-${parsedParam.revisionNumber}`, + }), + }, + component: Outlet, + }) + const parsedParamIndexRoute = createRoute({ + getParentRoute: () => parsedParamRoute, + path: '/', + loader: ({ params }) => params.parsedParam.revisionNumber * 100, + component: () => { + const params = parsedParamIndexRoute.useParams() + const loaderData = parsedParamIndexRoute.useLoaderData() + + return ( + <> +
+ Params have {params.parsedParam.recordName} with revision{' '} + {params.parsedParam.revisionNumber} +
+
Loader data is {loaderData}
+ + ) + }, + }) + const routeTree = rootRoute.addChildren([ + indexRoute, + parsedParamRoute.addChildren([parsedParamIndexRoute]), + ]) + const router = createRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + render() + expect(await screen.findByText('Home page')).toBeInTheDocument() + + await act(() => + router.navigate({ + to: '/$parsedParam', + params: { parsedParam: { recordName: 'blue', revisionNumber: 1 } }, + }), + ) + expect( + await screen.findByText('Params have blue with revision 1'), + ).toBeInTheDocument() + expect(await screen.findByText('Loader data is 100')).toBeInTheDocument() + + await act(() => router.navigate({ to: '/' })) + expect(await screen.findByText('Home page')).toBeInTheDocument() + + await act(() => + router.navigate({ + to: '/$parsedParam', + params: { parsedParam: { recordName: 'red', revisionNumber: 2 } }, + }), + ) + expect(router.state.location.pathname).toBe('/red-2') + + await waitFor(() => { + expect({ + params: screen.getByTestId('params').textContent, + loaderData: screen.getByTestId('loader-data').textContent, + }).toEqual({ + params: 'Params have red with revision 2', + loaderData: 'Loader data is 200', + }) + }) +}) diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 5b0cbfb6be6..74b6f015a8f 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -750,13 +750,12 @@ export type ParseLocationFn = ( previousLocation?: ParsedLocation>, ) => ParsedLocation> -export type GetMatchRoutesFn = (pathname: string) => { - matchedRoutes: ReadonlyArray +export type GetMatchRoutesFn = (pathname: string) => [ + matchedRoutes: ReadonlyArray, /** exhaustive params, still in their string form */ - routeParams: Record - foundRoute: AnyRoute | undefined - parseError?: unknown -} + rawParams: Record, + foundRoute: AnyRoute | undefined, +] export type EmitFn = (routerEvent: RouterEvent) => void @@ -1512,16 +1511,17 @@ export class RouterCore< next: ParsedLocation, opts?: MatchRoutesOpts, ): Array { - const matchedRoutesResult = this.getMatchedRoutes(next.pathname) - const { foundRoute, routeParams } = matchedRoutesResult - let { matchedRoutes } = matchedRoutesResult + const [initialMatchedRoutes, rawParams, foundRoute] = this.getMatchedRoutes( + next.pathname, + ) + let matchedRoutes = initialMatchedRoutes let isGlobalNotFound = false // Check to see if the route needs a 404 entry if ( // If we found a route, and it's not an index route and we have left over path foundRoute - ? foundRoute.path !== '/' && routeParams['**'] + ? foundRoute.path !== '/' && rawParams['**'] : // Or if we didn't find a route and we have left over path trimPathRight(next.pathname) ) { @@ -1549,6 +1549,7 @@ export class RouterCore< : undefined } + let strictParams: AnyRouteMatch['_strictParams'] | undefined for (let index = 0; index < matchedRoutes.length; index++) { const route = matchedRoutes[index]! // Take each matched route and resolve + validate its search params @@ -1614,9 +1615,10 @@ export class RouterCore< } searchError ??= cause } + // Match identity must only use the raw params captured from the URL. const { interpolatedPath, usedParams } = interpolatePath({ path: route.fullPath, - params: routeParams, + params: rawParams, decoder: this.pathParamsDecoder, server: this.isServer, }) @@ -1639,7 +1641,9 @@ export class RouterCore< : (this._cache.get(matchId) ?? (previousMatch?.id === matchId ? previousMatch : undefined)) - const strictParams = existingMatch?._strictParams ?? usedParams + // Carry parsed ancestors forward without mutating the raw route params. + strictParams = + existingMatch?._strictParams ?? Object.assign(usedParams, strictParams) let paramsError: unknown @@ -1661,8 +1665,6 @@ export class RouterCore< } } - Object.assign(routeParams, strictParams) - const cause = previousMatch ? 'stay' : 'enter' let match: AnyRouteMatch @@ -1671,8 +1673,6 @@ export class RouterCore< match = { ...existingMatch, cause, - params: previousMatch?.params ?? routeParams, - _strictParams: strictParams, search: previousMatch ? nullReplaceEqualDeep(previousMatch.search, preMatchSearch) : nullReplaceEqualDeep(existingMatch.search, preMatchSearch), @@ -1687,7 +1687,7 @@ export class RouterCore< ssr: (isServer ?? this.isServer) ? undefined : route.options.ssr, index, routeId: route.id, - params: previousMatch?.params ?? routeParams, + params: previousMatch?.params ?? strictParams, _strictParams: strictParams, pathname: interpolatedPath, updatedAt: Date.now(), @@ -1727,8 +1727,8 @@ export class RouterCore< const match = matches[index]! match.params = match.cause === 'stay' - ? nullReplaceEqualDeep(match.params, routeParams) - : routeParams + ? nullReplaceEqualDeep(match.params, strictParams) + : strictParams! if (opts?._controller) { match.context = {} } @@ -1738,20 +1738,20 @@ export class RouterCore< } getMatchedRoutes: GetMatchRoutesFn = (pathname) => { - const routeParams: Record = Object.create(null) + const rawParams: Record = Object.create(null) const match = findRouteMatch( trimPathRight(pathname), this.processedTree, true, ) if (match) { - Object.assign(routeParams, match.rawParams) - } - return { - matchedRoutes: match?.branch || [this.routesById[rootRouteId]!], - routeParams, - foundRoute: match?.route, + Object.assign(rawParams, match.rawParams) } + return [ + match?.branch || [this.routesById[rootRouteId]!], + rawParams, + match?.route, + ] } /** @@ -1772,9 +1772,7 @@ export class RouterCore< return cached[1 /* result */] } - const { matchedRoutes, routeParams } = this.getMatchedRoutes( - location.pathname, - ) + const [matchedRoutes, rawParams] = this.getMatchedRoutes(location.pathname) const lastRoute = last(matchedRoutes)! // I don't know if we should run the full search middleware chain, or just validateSearch @@ -1812,7 +1810,7 @@ export class RouterCore< // Parse params through the route chain const strictParams: Record = Object.assign( Object.create(null), - routeParams, + rawParams, ) for (const route of matchedRoutes) { try { @@ -1862,7 +1860,7 @@ export class RouterCore< process.env.NODE_ENV !== 'production' && dest._isNavigate ) { - const allFromMatches = this.getMatchedRoutes(dest.from).matchedRoutes + const [allFromMatches] = this.getMatchedRoutes(dest.from) const matchedFrom = findLast(lightweightResult.matchedRoutes, (d) => { return comparePaths(d.fullPath, dest.from!) @@ -1918,14 +1916,13 @@ export class RouterCore< // typed destination mismatch, not a concrete URL to route-match. destRoutes = [] } else { - const destMatchResult = this.getMatchedRoutes(nextTo) - destRoutes = destMatchResult.matchedRoutes + const [matchedRoutes, rawParams, foundRoute] = + this.getMatchedRoutes(nextTo) + destRoutes = matchedRoutes if ( this.options.notFoundRoute && - (!destMatchResult.foundRoute || - (destMatchResult.foundRoute.path !== '/' && - destMatchResult.routeParams['**'])) + (!foundRoute || (foundRoute.path !== '/' && rawParams['**'])) ) { destRoutes = [...destRoutes, this.options.notFoundRoute] } @@ -1968,10 +1965,10 @@ export class RouterCore< !opts.leaveParams ) { try { - const roundTrip = this.getMatchedRoutes(nextPathname) - if (roundTrip.foundRoute?.id !== destRoute.id) { + const foundRoute = this.getMatchedRoutes(nextPathname)[2] + if (foundRoute?.id !== destRoute.id) { console.warn( - `Generated path "${nextPathname}" for route "${destRoute.id}" matched route "${roundTrip.foundRoute?.id}" instead. This can happen when multiple route templates resolve to the same URL. Use the route template that matches the intended route, or adjust params.stringify if it changed the target path.`, + `Generated path "${nextPathname}" for route "${destRoute.id}" matched route "${foundRoute?.id}" instead. This can happen when multiple route templates resolve to the same URL. Use the route template that matches the intended route, or adjust params.stringify if it changed the target path.`, ) } } catch { diff --git a/packages/router-plugin/tests/handle-route-update.test.ts b/packages/router-plugin/tests/handle-route-update.test.ts index db371746af4..9c8e239fe27 100644 --- a/packages/router-plugin/tests/handle-route-update.test.ts +++ b/packages/router-plugin/tests/handle-route-update.test.ts @@ -240,7 +240,7 @@ describe('handleRouteUpdate', () => { const routeTree = rootRoute.addChildren([asAnyRoute(itemRoute)]) const router = createTestRouter(routeTree) - expect(router.getMatchedRoutes('/items/abc').foundRoute?.id).toBeUndefined() + expect(router.getMatchedRoutes('/items/abc')[2]?.id).toBeUndefined() const restoreWindow = withWindowRouter(router) try { @@ -258,9 +258,7 @@ describe('handleRouteUpdate', () => { restoreWindow() } - expect(router.getMatchedRoutes('/items/abc').foundRoute?.id).toBe( - itemRoute.id, - ) + expect(router.getMatchedRoutes('/items/abc')[2]?.id).toBe(itemRoute.id) }) it('hydrates the hot module route export with generated route tree state', () => { diff --git a/packages/solid-router/src/useBlocker.tsx b/packages/solid-router/src/useBlocker.tsx index 2e87ed0673e..566a58d1ca4 100644 --- a/packages/solid-router/src/useBlocker.tsx +++ b/packages/solid-router/src/useBlocker.tsx @@ -186,21 +186,23 @@ export function useBlocker( location: HistoryLocation, ): AnyShouldBlockFnLocation { const parsedLocation = router.parseLocation(location) - const matchedRoutes = router.getMatchedRoutes(parsedLocation.pathname) - if (matchedRoutes.foundRoute === undefined) { + const [, rawParams, foundRoute] = router.getMatchedRoutes( + parsedLocation.pathname, + ) + if (foundRoute === undefined) { return { routeId: '__notFound__', fullPath: parsedLocation.pathname, pathname: parsedLocation.pathname, - params: matchedRoutes.routeParams, + params: rawParams, search: parsedLocation.search, } } return { - routeId: matchedRoutes.foundRoute.id, - fullPath: matchedRoutes.foundRoute.fullPath, + routeId: foundRoute.id, + fullPath: foundRoute.fullPath, pathname: parsedLocation.pathname, - params: matchedRoutes.routeParams, + params: rawParams, search: parsedLocation.search, } } diff --git a/packages/solid-router/tests/issue-7964-param-parsing-loader.test.tsx b/packages/solid-router/tests/issue-7964-param-parsing-loader.test.tsx new file mode 100644 index 00000000000..ca49ad1dc7b --- /dev/null +++ b/packages/solid-router/tests/issue-7964-param-parsing-loader.test.tsx @@ -0,0 +1,102 @@ +import { cleanup, render, screen, waitFor } from '@solidjs/testing-library' +import { afterEach, expect, test } from 'vitest' +import { + Outlet, + RouterProvider, + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, +} from '../src' + +afterEach(() => { + cleanup() +}) + +// https://github.com/TanStack/router/issues/7964 +test('#7964: a child loader receives fresh structured params after revisiting its route', async () => { + const rootRoute = createRootRoute({ + component: Outlet, + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + component: () =>
Home page
, + }) + const parsedParamRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/$parsedParam', + params: { + parse: ({ parsedParam }) => { + const [recordName, revisionNumber] = parsedParam.split('-') + return { + parsedParam: { + recordName: recordName!, + revisionNumber: Number(revisionNumber), + }, + } + }, + stringify: ({ parsedParam }) => ({ + parsedParam: `${parsedParam.recordName}-${parsedParam.revisionNumber}`, + }), + }, + component: Outlet, + }) + const parsedParamIndexRoute = createRoute({ + getParentRoute: () => parsedParamRoute, + path: '/', + loader: ({ params }) => params.parsedParam.revisionNumber * 100, + component: () => { + const params = parsedParamIndexRoute.useParams() + const loaderData = parsedParamIndexRoute.useLoaderData() + + return ( + <> +
+ Params have {params().parsedParam.recordName} with revision{' '} + {params().parsedParam.revisionNumber} +
+
Loader data is {loaderData()}
+ + ) + }, + }) + const routeTree = rootRoute.addChildren([ + indexRoute, + parsedParamRoute.addChildren([parsedParamIndexRoute]), + ]) + const router = createRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + render(() => ) + + await router.navigate({ + to: '/$parsedParam', + params: { parsedParam: { recordName: 'blue', revisionNumber: 1 } }, + }) + expect( + await screen.findByText('Params have blue with revision 1'), + ).toBeInTheDocument() + expect(await screen.findByText('Loader data is 100')).toBeInTheDocument() + + await router.navigate({ to: '/' }) + expect(await screen.findByText('Home page')).toBeInTheDocument() + + await router.navigate({ + to: '/$parsedParam', + params: { parsedParam: { recordName: 'red', revisionNumber: 2 } }, + }) + expect(router.state.location.pathname).toBe('/red-2') + + await waitFor(() => { + expect({ + params: screen.getByTestId('params').textContent, + loaderData: screen.getByTestId('loader-data').textContent, + }).toEqual({ + params: 'Params have red with revision 2', + loaderData: 'Loader data is 200', + }) + }) +}) diff --git a/packages/start-server-core/src/createStartHandler.ts b/packages/start-server-core/src/createStartHandler.ts index 20c47631379..c7b427bfb1f 100644 --- a/packages/start-server-core/src/createStartHandler.ts +++ b/packages/start-server-core/src/createStartHandler.ts @@ -893,10 +893,10 @@ async function handleServerRoutes({ // this will perform a fuzzy match, however for server routes we need an exact match // if the route is not an exact match, executeRouter will handle rendering the app router // the match will be cached internally, so no extra work is done during the app router render - const { matchedRoutes, foundRoute, routeParams } = + const [matchedRoutes, rawParams, foundRoute] = router.getMatchedRoutes(pathname) - const isExactMatch = foundRoute && routeParams['**'] === undefined + const isExactMatch = foundRoute && rawParams['**'] === undefined // Collect and dedupe route middlewares const routeMiddlewares: Array = [] @@ -964,7 +964,7 @@ async function handleServerRoutes({ { request, context, - params: routeParams, + params: rawParams, pathname, handlerType: 'router', }, diff --git a/packages/vue-router/src/useBlocker.tsx b/packages/vue-router/src/useBlocker.tsx index 4802a3e82af..17eef20f7aa 100644 --- a/packages/vue-router/src/useBlocker.tsx +++ b/packages/vue-router/src/useBlocker.tsx @@ -179,21 +179,23 @@ export function useBlocker( location: HistoryLocation, ): AnyShouldBlockFnLocation { const parsedLocation = router.parseLocation(location) - const matchedRoutes = router.getMatchedRoutes(parsedLocation.pathname) - if (matchedRoutes.foundRoute === undefined) { + const [, rawParams, foundRoute] = router.getMatchedRoutes( + parsedLocation.pathname, + ) + if (foundRoute === undefined) { return { routeId: '__notFound__', fullPath: parsedLocation.pathname, pathname: parsedLocation.pathname, - params: matchedRoutes.routeParams, + params: rawParams, search: parsedLocation.search, } } return { - routeId: matchedRoutes.foundRoute.id, - fullPath: matchedRoutes.foundRoute.fullPath, + routeId: foundRoute.id, + fullPath: foundRoute.fullPath, pathname: parsedLocation.pathname, - params: matchedRoutes.routeParams, + params: rawParams, search: parsedLocation.search, } } @@ -373,21 +375,23 @@ const BlockImpl = Vue.defineComponent({ location: HistoryLocation, ): AnyShouldBlockFnLocation { const parsedLocation = router.parseLocation(location) - const matchedRoutes = router.getMatchedRoutes(parsedLocation.pathname) - if (matchedRoutes.foundRoute === undefined) { + const [, rawParams, foundRoute] = router.getMatchedRoutes( + parsedLocation.pathname, + ) + if (foundRoute === undefined) { return { routeId: '__notFound__', fullPath: parsedLocation.pathname, pathname: parsedLocation.pathname, - params: matchedRoutes.routeParams, + params: rawParams, search: parsedLocation.search, } } return { - routeId: matchedRoutes.foundRoute.id, - fullPath: matchedRoutes.foundRoute.fullPath, + routeId: foundRoute.id, + fullPath: foundRoute.fullPath, pathname: parsedLocation.pathname, - params: matchedRoutes.routeParams, + params: rawParams, search: parsedLocation.search, } }