diff --git a/.changeset/heavy-donkeys-wave.md b/.changeset/heavy-donkeys-wave.md new file mode 100644 index 0000000000..2c437cc421 --- /dev/null +++ b/.changeset/heavy-donkeys-wave.md @@ -0,0 +1,8 @@ +--- +'@tanstack/router-core': patch +'@tanstack/react-router': patch +'@tanstack/solid-router': patch +'@tanstack/vue-router': patch +--- + +Fix path param values starting with `$` being rewritten to "undefined" on full page load. The canonical-URL check on mount and on server startup no longer treats the current concrete pathname as a route template. diff --git a/packages/react-router/src/Transitioner.tsx b/packages/react-router/src/Transitioner.tsx index bf945571a6..c91045796a 100644 --- a/packages/react-router/src/Transitioner.tsx +++ b/packages/react-router/src/Transitioner.tsx @@ -43,6 +43,7 @@ export function Transitioner() { hash: true, state: true, _includeValidateSearch: true, + _concreteTo: true, }) // Check if the current URL matches the canonical form. diff --git a/packages/react-router/tests/router.test.tsx b/packages/react-router/tests/router.test.tsx index cf3b1c11fe..e10eea98e3 100644 --- a/packages/react-router/tests/router.test.tsx +++ b/packages/react-router/tests/router.test.tsx @@ -388,6 +388,27 @@ describe('encoding: path params', () => { expect(router.state.location.pathname).toBe('/posts/100%2525') }) + it('should preserve a param value starting with $ on initial load', async () => { + const { router } = createTestRouter({ + history, + pathParamsAllowedCharacters: ['$'], + }) + window.history.replaceState(null, '', '/posts/$EXAMPLE_CODE%2Ffile.abap') + + render() + await act(() => router.load()) + + // The mount-time canonical-URL check must not treat the concrete + // pathname as a route template and rewrite it to /posts/undefined + expect(window.location.pathname).toBe('/posts/$EXAMPLE_CODE%2Ffile.abap') + expect(router.state.location.pathname).toBe( + '/posts/$EXAMPLE_CODE%2Ffile.abap', + ) + expect(router.state.matches.at(-1)?.params).toEqual({ + slug: '$EXAMPLE_CODE/file.abap', + }) + }) + describe('pathname and URI syntax characters', () => { it.each(URISyntaxCharacters)( 'pathname should encode $0', diff --git a/packages/router-core/src/RouterProvider.ts b/packages/router-core/src/RouterProvider.ts index 52aeaffabb..781a85d4ad 100644 --- a/packages/router-core/src/RouterProvider.ts +++ b/packages/router-core/src/RouterProvider.ts @@ -43,5 +43,6 @@ export type BuildLocationFn = < leaveParams?: boolean _includeValidateSearch?: boolean _isNavigate?: boolean + _concreteTo?: boolean }, ) => ParsedLocation diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 9ff7b82b05..7ede86d890 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -573,6 +573,14 @@ export interface BuildNextOptions { _fromLocation?: ParsedLocation unsafeRelative?: 'path' _isNavigate?: boolean + /** + * @internal + * Marks `to` as a concrete pathname (e.g. the current location being + * rebuilt on load) instead of a route template. Concrete pathnames are + * route-matched on a miss and never interpolated, so segments that merely + * look like params (`/$foo`) are left untouched. + */ + _concreteTo?: boolean } type NavigationEventInfo = { @@ -1906,7 +1914,7 @@ export class RouterCore< let destRoutes: ReadonlyArray if (destRoute) { destRoutes = this.getRouteBranch(destRoute) - } else if (nextTo.includes('$')) { + } else if (!dest._concreteTo && nextTo.includes('$')) { // Route templates must match routesByPath exactly. A miss here is a // typed destination mismatch, not a concrete URL to route-match. destRoutes = [] @@ -1943,17 +1951,21 @@ export class RouterCore< } } - const nextPathname = opts.leaveParams - ? // Keep path params uninterpolated for matchRoute/template matching. - nextTo - : decodePath( - interpolatePath({ - path: nextTo, - params: nextParams, - decoder: this.pathParamsDecoder, - server: this.isServer, - }).interpolatedPath, - ).path + const nextPathname = + opts.leaveParams || dest._concreteTo + ? // Keep path params uninterpolated for matchRoute/template matching. + // Concrete pathnames have nothing to interpolate either, and + // treating them as templates would mangle segments that look + // like params. + nextTo + : decodePath( + interpolatePath({ + path: nextTo, + params: nextParams, + decoder: this.pathParamsDecoder, + server: this.isServer, + }).interpolatedPath, + ).path if ( process.env.NODE_ENV !== 'production' && @@ -2408,6 +2420,7 @@ export class RouterCore< hash: true, state: true, _includeValidateSearch: true, + _concreteTo: true, }) // Check if location changed - origin check is unnecessary since buildLocation diff --git a/packages/router-core/tests/build-location.test.ts b/packages/router-core/tests/build-location.test.ts index 131fccf095..7a049cbb6d 100644 --- a/packages/router-core/tests/build-location.test.ts +++ b/packages/router-core/tests/build-location.test.ts @@ -2295,3 +2295,77 @@ describe('buildLocation - _fromLocation override', () => { expect(location.pathname).toBe('/users/456/settings') }) }) + +describe('buildLocation - _concreteTo treats `to` as a concrete pathname', () => { + function setup(initialEntry: string) { + const rootRoute = new BaseRootRoute({}) + const fileRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/files/$filePath', + validateSearch: (search: Record) => ({ + internal: search.internal, + }), + search: { + middlewares: [stripSearchParams(['internal'])], + }, + }) + const routeTree = rootRoute.addChildren([fileRoute]) + + const router = createTestRouter({ + routeTree, + pathParamsAllowedCharacters: ['$'], + history: createMemoryHistory({ initialEntries: [initialEntry] }), + }) + + return router + } + + // Mirrors the canonical-URL check that Transitioner (on mount) and + // router.beforeLoad (on the server) perform for the current location. + function buildCurrentLocation(router: ReturnType) { + return router.buildLocation({ + to: router.latestLocation.pathname, + search: true, + params: true, + hash: true, + state: true, + _includeValidateSearch: true, + _concreteTo: true, + } as any) + } + + test('param value starting with $ survives the reload round-trip', async () => { + const router = setup('/files/$EXAMPLE_CODE%2Ffile.abap') + + await router.load() + + const location = buildCurrentLocation(router) + + expect(location.pathname).toBe('/files/$EXAMPLE_CODE%2Ffile.abap') + expect(location.publicHref).toBe(router.latestLocation.publicHref) + }) + + test('search middlewares run for a concrete pathname containing $', async () => { + const router = setup('/files/$EXAMPLE_CODE?internal=1') + + await router.load() + + const location = buildCurrentLocation(router) + + expect(location.pathname).toBe('/files/$EXAMPLE_CODE') + expect(location.search).toEqual({}) + }) + + test('without _concreteTo, `to` is still treated as a route template', async () => { + const router = setup('/files/readme.txt') + + await router.load() + + const location = router.buildLocation({ + to: '/files/$filePath', + params: { filePath: 'other.txt' }, + }) + + expect(location.pathname).toBe('/files/other.txt') + }) +}) diff --git a/packages/router-core/tests/load.test.ts b/packages/router-core/tests/load.test.ts index 1ea6fca30e..4d833654c7 100644 --- a/packages/router-core/tests/load.test.ts +++ b/packages/router-core/tests/load.test.ts @@ -54,7 +54,7 @@ describe('redirect resolution', () => { }) test.each(['/$a', '/$toString', '/$__proto__'])( - 'server startup redirects initial path %s to /undefined', + 'server startup preserves initial path %s and matches it as a param value', async (initialPath) => { const rootRoute = new BaseRootRoute({}) const slugRoute = new BaseRoute({ @@ -72,12 +72,14 @@ describe('redirect resolution', () => { await router.load() - expect(router.state.redirect).toEqual( - expect.objectContaining({ - options: expect.objectContaining({ href: '/undefined' }), - }), - ) - expect(router.state.redirect?.headers.get('Location')).toBe('/undefined') + // The concrete pathname must not be treated as a route template, so + // there is nothing to canonicalize and no redirect. Inherited object + // properties (toString, __proto__) must not leak into the result. + expect(router.state.redirect).toBeUndefined() + expect(router.state.location.pathname).toBe(initialPath) + expect(router.state.matches.at(-1)?.params).toEqual({ + slug: initialPath.slice(1), + }) }, ) }) diff --git a/packages/solid-router/src/Transitioner.tsx b/packages/solid-router/src/Transitioner.tsx index 331aff5864..acc8374f07 100644 --- a/packages/solid-router/src/Transitioner.tsx +++ b/packages/solid-router/src/Transitioner.tsx @@ -41,6 +41,7 @@ export function Transitioner() { hash: true, state: true, _includeValidateSearch: true, + _concreteTo: true, }) // Check if the current URL matches the canonical form. diff --git a/packages/vue-router/src/Transitioner.tsx b/packages/vue-router/src/Transitioner.tsx index b0133d965c..62f4a9c52f 100644 --- a/packages/vue-router/src/Transitioner.tsx +++ b/packages/vue-router/src/Transitioner.tsx @@ -112,6 +112,7 @@ export function useTransitionerSetup() { hash: true, state: true, _includeValidateSearch: true, + _concreteTo: true, }) // Check if the current URL matches the canonical form.