diff --git a/.changeset/polite-gifts-camp.md b/.changeset/polite-gifts-camp.md new file mode 100644 index 00000000000..2df1f6f23af --- /dev/null +++ b/.changeset/polite-gifts-camp.md @@ -0,0 +1,6 @@ +--- +'@tanstack/router-core': patch +'@tanstack/start-server-core': patch +--- + +Reuse the parsed request location during SSR instead of repeating input rewrites. Match server routes against the app router's parsed pathname while preserving encoded pathnames for server handlers and middleware. diff --git a/packages/router-core/src/load-server.ts b/packages/router-core/src/load-server.ts index aff9e11d269..325f8d63f7d 100644 --- a/packages/router-core/src/load-server.ts +++ b/packages/router-core/src/load-server.ts @@ -904,7 +904,9 @@ export async function loadServerRoute( router: AnyRouter, opts?: ServerLoadOptions, ): Promise { - router.updateLatestLocation() + if (!opts?._skipLocationUpdate) { + router.updateLatestLocation() + } const next = router.latestLocation const previous = router._committed const previousEnd = router._lifecycleEnd diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 1974690b39a..fa49039b22e 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -788,6 +788,8 @@ export type LoadFn = (opts?: { sync?: boolean action?: { type: HistoryAction } _signal?: AbortSignal + /** @private Reuse the location already prepared for this SSR request. */ + _skipLocationUpdate?: boolean }) => Promise export type CommitLocationFn = ({ diff --git a/packages/router-core/src/ssr/createRequestHandler.ts b/packages/router-core/src/ssr/createRequestHandler.ts index e3297730d0c..21e48ca2426 100644 --- a/packages/router-core/src/ssr/createRequestHandler.ts +++ b/packages/router-core/src/ssr/createRequestHandler.ts @@ -72,6 +72,7 @@ export function createRequestHandler({ await router.load({ _signal: signal, + _skipLocationUpdate: true, }) signal.throwIfAborted() diff --git a/packages/router-core/tests/server-history.test.ts b/packages/router-core/tests/server-history.test.ts index 86485fbe4c9..6343a08cd2a 100644 --- a/packages/router-core/tests/server-history.test.ts +++ b/packages/router-core/tests/server-history.test.ts @@ -1,8 +1,142 @@ import { createMemoryHistory, createServerHistory } from '@tanstack/history' import { expect, test, vi } from 'vitest' import { BaseRootRoute, BaseRoute, redirect } from '../src' +import { createRequestHandler } from '../src/ssr/server' import { createTestRouter, loadServerResponse } from './routerTestUtils' +test.each([ + { basepath: '', origin: undefined }, + { basepath: '/app', origin: undefined }, + { basepath: '/app', origin: 'https://canonical.example' }, +])( + 'request handling rewrites once with basepath $basepath and origin $origin', + async ({ basepath, origin }) => { + const root = new BaseRootRoute() + const input = vi.fn(({ url }: { url: URL }) => { + url.pathname = url.pathname.replace('/public/', '/posts/') + return url + }) + const router = createTestRouter({ + isServer: true, + basepath, + origin, + rewrite: { + input, + output: ({ url }) => { + url.pathname = url.pathname.replace('/posts/', '/public/') + return url + }, + }, + routeTree: root.addChildren([ + new BaseRoute({ + getParentRoute: () => root, + path: '/posts/$postId', + loader: ({ params }) => params.postId, + }), + ]), + }) + input.mockClear() + + const response = await createRequestHandler({ + createRouter: () => router, + request: new Request( + `https://example.com${basepath}/public/caf%C3%A9?view=full`, + ), + })(({ router: loadedRouter }) => { + return Response.json({ + postId: loadedRouter.state.matches.at(-1)?.loaderData, + pathname: loadedRouter.state.location.pathname, + search: loadedRouter.state.location.search, + }) + }) + + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ + postId: 'café', + pathname: '/posts/café', + search: { view: 'full' }, + }) + expect(input).toHaveBeenCalledTimes(1) + expect(input.mock.calls[0]?.[0].url.origin).toBe( + origin ?? 'https://example.com', + ) + }, +) + +test.each([ + { isServer: false, action: 'push' }, + { isServer: false, action: 'replace' }, + { isServer: true, action: 'push' }, + { isServer: true, action: 'replace' }, +] as const)( + 'loads refresh memory history after $action with isServer=$isServer by default', + async ({ isServer, action }) => { + const root = new BaseRootRoute() + const targetLoader = vi.fn(() => 'target') + const history = createMemoryHistory({ initialEntries: ['/'] }) + const router = createTestRouter({ + isServer, + history, + routeTree: root.addChildren([ + new BaseRoute({ getParentRoute: () => root, path: '/' }), + new BaseRoute({ + getParentRoute: () => root, + path: '/target', + loader: targetLoader, + }), + ]), + }) + await router.load() + + history[action]('/target') + await router.load() + + expect(router.state.location.pathname).toBe('/target') + expect(targetLoader).toHaveBeenCalledTimes(1) + expect(router.state.matches.at(-1)?.loaderData).toBe('target') + }, +) + +test('server loads reuse a location prepared with updated router options', async () => { + const root = new BaseRootRoute() + const input = vi.fn(({ url }: { url: URL }) => { + url.pathname = url.pathname.replace('/public', '/target') + return url + }) + const router = createTestRouter({ + isServer: true, + basepath: '/old', + history: createMemoryHistory({ initialEntries: ['/old/'] }), + routeTree: root.addChildren([ + new BaseRoute({ getParentRoute: () => root, path: '/' }), + new BaseRoute({ + getParentRoute: () => root, + path: '/target', + loader: () => 'target', + }), + ]), + }) + router.update({ + history: createServerHistory('/app/public'), + basepath: '/app', + rewrite: { + input, + output: ({ url }) => { + url.pathname = url.pathname.replace('/target', '/public') + return url + }, + }, + }) + input.mockClear() + + await router.load({ _skipLocationUpdate: true }) + + expect(router.state.location.pathname).toBe('/target') + expect(router.state.location.publicHref).toBe('/app/public') + expect(router.state.matches.at(-1)?.loaderData).toBe('target') + expect(input).not.toHaveBeenCalled() +}) + test.each([false, true])( 'router navigation respects isServer=%s with memory history', async (isServer) => { diff --git a/packages/start-server-core/src/createStartHandler.ts b/packages/start-server-core/src/createStartHandler.ts index 84af2d4558f..2547318a34c 100644 --- a/packages/start-server-core/src/createStartHandler.ts +++ b/packages/start-server-core/src/createStartHandler.ts @@ -9,7 +9,6 @@ import { } from '@tanstack/start-client-core' import { _getRenderedMatches, - executeRewriteInput, isDangerousProtocol, isPromise, isRedirect, @@ -729,7 +728,10 @@ export function createStartHandler( // `additionalContext` is request-scoped and only read from router.options // during load; avoid a full router.update() and redundant location parse. routerInstance.options.additionalContext = { serverContext } - await routerInstance.load({ _signal: signal }) + await routerInstance.load({ + _signal: signal, + _skipLocationUpdate: true, + }) signal.throwIfAborted() if (routerInstance._serverResult?.type === 'redirect') { @@ -774,7 +776,6 @@ export function createStartHandler( handleServerRoutes({ getRouter, request, - url, executeRouter, context, executedRequestMiddlewares, @@ -909,14 +910,12 @@ async function handleRedirectResponse( async function handleServerRoutes({ getRouter, request, - url, executeRouter, context, executedRequestMiddlewares, }: { getRouter: () => Promise request: Request - url: URL executeRouter: ( serverContext: any, matchedRoutes?: ReadonlyArray, @@ -925,13 +924,15 @@ async function handleServerRoutes({ executedRequestMiddlewares: Set }): Promise { const router = await getRouter() - const rewrittenUrl = executeRewriteInput(router.rewrite, url) - const pathname = rewrittenUrl.pathname + const location = router.latestLocation + // Preserve the encoded pathname exposed to server handlers and middleware. + const pathname = location.href.split(/[?#]/, 1)[0]! // 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, rawParams, foundRoute] = - router.getMatchedRoutes(pathname) + // The cached match avoids another route-tree traversal during the app router render. + const [matchedRoutes, rawParams, foundRoute] = router.getMatchedRoutes( + location.pathname, + ) const isExactMatch = foundRoute && rawParams['**'] === undefined diff --git a/packages/start-server-core/tests/createStartHandler.test.ts b/packages/start-server-core/tests/createStartHandler.test.ts index 18bbbcb9f28..2276b140394 100644 --- a/packages/start-server-core/tests/createStartHandler.test.ts +++ b/packages/start-server-core/tests/createStartHandler.test.ts @@ -582,6 +582,219 @@ describe('createStartHandler redirect safety', () => { ) }) +describe('createStartHandler request location reuse', () => { + it.each( + ['plain', 'café'].flatMap((path) => + [false, true].map((renderApp) => ({ path, renderApp })), + ), + )( + 'rewrites once for $path (renderApp=$renderApp)', + async ({ path, renderApp }) => { + const input = vi.fn(({ url }: { url: URL }) => { + url.pathname = url.pathname.replace('/public/', '/') + return url + }) + const middlewarePathnames: Array = [] + const routeMiddleware = createMiddleware().server( + ({ pathname, next }) => { + middlewarePathnames.push(pathname) + return next() + }, + ) + const serverHandler = vi.fn() + const rootRoute = new BaseRootRoute({}) + const route = new BaseRoute({ + getParentRoute: () => rootRoute, + path, + component: () => null, + server: { + middleware: [routeMiddleware], + handlers: { + GET: ({ pathname, next }) => { + serverHandler(pathname) + return renderApp ? next() : new Response('server response') + }, + }, + }, + }) + const router = new RouterCore( + { + history: createMemoryHistory({ initialEntries: ['/'] }), + routeTree: rootRoute.addChildren([route]), + rewrite: { + input, + output: ({ url }) => { + url.pathname = `/public${url.pathname}` + return url + }, + }, + }, + getStoreConfig, + ) + router.isServer = true + startMocks.router = router + input.mockClear() + const getMatchedRoutes = vi.spyOn(router, 'getMatchedRoutes') + const render = vi.fn(() => new Response('app response')) + const handler = createStartHandler(render) + + const response = await handler( + new Request( + `http://localhost/public/${encodeURIComponent(path)}?view=full#section`, + ), + {}, + ) + + expect(response.status).toBe(200) + expect(await response.text()).toBe( + renderApp ? 'app response' : 'server response', + ) + expect(input).toHaveBeenCalledOnce() + expect(serverHandler).toHaveBeenCalledExactlyOnceWith( + `/${encodeURIComponent(path)}`, + ) + expect(middlewarePathnames).toEqual([`/${encodeURIComponent(path)}`]) + expect(render).toHaveBeenCalledTimes(renderApp ? 1 : 0) + expect( + getMatchedRoutes.mock.calls.every( + ([pathname]) => pathname === `/${path}`, + ), + ).toBe(true) + if (renderApp) { + expect(router.state.matches.at(-1)?.routeId).toBe(route.id) + } + }, + ) + + it('uses the configured origin for server route rewrites', async () => { + const input = vi.fn(({ url }: { url: URL }) => { + url.pathname = + url.hostname === 'public.example' ? '/work' : '/wrong-origin' + return url + }) + const router = makeRouterWithRouteWork({}) + router.update({ + origin: 'https://public.example', + rewrite: { + input, + output: ({ url }) => { + url.pathname = '/public' + return url + }, + }, + }) + const serverHandler = vi.fn(() => new Response('server response')) + router.routesById['/work']!.options.server = { + handlers: { GET: serverHandler }, + } + startMocks.router = router + input.mockClear() + const handler = createStartHandler(() => new Response('app response')) + + const response = await handler( + new Request('http://internal.example/public'), + {}, + ) + + expect(response.status).toBe(200) + expect(await response.text()).toBe('server response') + expect(serverHandler).toHaveBeenCalledOnce() + expect(input).toHaveBeenCalledOnce() + }) + + it('reuses search parsed after route middleware updates the router', async () => { + const parseSearch = vi.fn((search: string) => + Object.fromEntries(new URLSearchParams(search)), + ) + const middleware = createMiddleware().server(({ next }) => { + startMocks.router!.update({ + parseSearch, + stringifySearch: (search) => + `?${new URLSearchParams(search).toString()}`, + }) + return next() + }) + const root = new BaseRootRoute() + const route = new BaseRoute({ + getParentRoute: () => root, + path: '/work', + component: () => null, + loader: ({ location }) => location.search, + server: { middleware: [middleware] }, + }) + startMocks.router = new RouterCore( + { isServer: true, routeTree: root.addChildren([route]) }, + getStoreConfig, + ) + const handler = createStartHandler(({ router }) => + Response.json({ + search: router.state.location.search, + loaderData: router.state.matches.at(-1)?.loaderData, + }), + ) + + const response = await handler( + new Request('http://localhost/work?page=2'), + {}, + ) + + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ + search: { page: '2' }, + loaderData: { page: '2' }, + }) + expect(parseSearch).toHaveBeenCalledExactlyOnceWith('?page=2') + }) + + it.each(['a/b', 'a%b', 'a b', 'a?b', 'a#b'])( + 'preserves encoded params %j when a server handler continues to SSR', + async (value) => { + const handlerParams: Array = [] + const handlerPathnames: Array = [] + const root = new BaseRootRoute() + const route = new BaseRoute({ + getParentRoute: () => root, + path: '/params/$value', + component: () => null, + loader: ({ params }) => params.value, + server: { + handlers: { + GET: ({ params, pathname, next }) => { + handlerParams.push(params.value) + handlerPathnames.push(pathname) + params.value = 'changed by server handler' + return next() + }, + }, + }, + }) + startMocks.router = new RouterCore( + { isServer: true, routeTree: root.addChildren([route]) }, + getStoreConfig, + ) + const handler = createStartHandler(({ router }) => + Response.json({ + params: router.state.matches.at(-1)?.params, + loaderData: router.state.matches.at(-1)?.loaderData, + }), + ) + + const response = await handler( + new Request(`http://localhost/params/${encodeURIComponent(value)}`), + {}, + ) + + expect(response.status).toBe(200) + expect(handlerParams).toEqual([value]) + expect(handlerPathnames).toEqual([`/params/${encodeURIComponent(value)}`]) + expect(await response.json()).toEqual({ + params: { value }, + loaderData: value, + }) + }, + ) +}) + it('keeps the request URL when server code attempts navigation', async () => { const loader = vi.fn(async () => { const router = startMocks.router!