diff --git a/.changeset/hungry-cooks-go.md b/.changeset/hungry-cooks-go.md new file mode 100644 index 00000000000..5bf60c3afc8 --- /dev/null +++ b/.changeset/hungry-cooks-go.md @@ -0,0 +1,5 @@ +--- +'@tanstack/start-server-core': patch +--- + +Apply route path parameter parsers, including parent-route parsers, before invoking server route handlers so runtime parameters agree with their inferred types. diff --git a/e2e/react-start/server-routes/src/routeTree.gen.ts b/e2e/react-start/server-routes/src/routeTree.gen.ts index 2454c3bb32c..7ae1e4f5510 100644 --- a/e2e/react-start/server-routes/src/routeTree.gen.ts +++ b/e2e/react-start/server-routes/src/routeTree.gen.ts @@ -20,7 +20,9 @@ import { Route as ApiOnlyAnyRouteImport } from './routes/api/only-any' import { Route as MethodsIndexRouteImport } from './routes/methods/index' import { Route as MethodsOnlyAnyRouteImport } from './routes/methods/only-any' import { Route as ApiParamsFooRouteRouteImport } from './routes/api/params/$foo/route' +import { Route as ApiParsedParamsIdRouteImport } from './routes/api/parsed-params.$id' import { Route as ApiParamsFooBarRouteImport } from './routes/api/params/$foo/$bar' +import { Route as ApiParsedParamsIdChildIdRouteImport } from './routes/api/parsed-params.$id.$childId' const IndexRoute = IndexRouteImport.update({ id: '/', @@ -77,11 +79,22 @@ const ApiParamsFooRouteRoute = ApiParamsFooRouteRouteImport.update({ path: '/api/params/$foo', getParentRoute: () => rootRouteImport, } as any) +const ApiParsedParamsIdRoute = ApiParsedParamsIdRouteImport.update({ + id: '/api/parsed-params/$id', + path: '/api/parsed-params/$id', + getParentRoute: () => rootRouteImport, +} as any) const ApiParamsFooBarRoute = ApiParamsFooBarRouteImport.update({ id: '/$bar', path: '/$bar', getParentRoute: () => ApiParamsFooRouteRoute, } as any) +const ApiParsedParamsIdChildIdRoute = + ApiParsedParamsIdChildIdRouteImport.update({ + id: '/$childId', + path: '/$childId', + getParentRoute: () => ApiParsedParamsIdRoute, + } as any) export interface FileRoutesByFullPath { '/': typeof IndexRoute @@ -95,7 +108,9 @@ export interface FileRoutesByFullPath { '/methods/only-any': typeof MethodsOnlyAnyRoute '/methods/': typeof MethodsIndexRoute '/api/params/$foo': typeof ApiParamsFooRouteRouteWithChildren + '/api/parsed-params/$id': typeof ApiParsedParamsIdRouteWithChildren '/api/params/$foo/$bar': typeof ApiParamsFooBarRoute + '/api/parsed-params/$id/$childId': typeof ApiParsedParamsIdChildIdRoute } export interface FileRoutesByTo { '/': typeof IndexRoute @@ -108,7 +123,9 @@ export interface FileRoutesByTo { '/methods/only-any': typeof MethodsOnlyAnyRoute '/methods': typeof MethodsIndexRoute '/api/params/$foo': typeof ApiParamsFooRouteRouteWithChildren + '/api/parsed-params/$id': typeof ApiParsedParamsIdRouteWithChildren '/api/params/$foo/$bar': typeof ApiParamsFooBarRoute + '/api/parsed-params/$id/$childId': typeof ApiParsedParamsIdChildIdRoute } export interface FileRoutesById { __root__: typeof rootRouteImport @@ -123,7 +140,9 @@ export interface FileRoutesById { '/methods/only-any': typeof MethodsOnlyAnyRoute '/methods/': typeof MethodsIndexRoute '/api/params/$foo': typeof ApiParamsFooRouteRouteWithChildren + '/api/parsed-params/$id': typeof ApiParsedParamsIdRouteWithChildren '/api/params/$foo/$bar': typeof ApiParamsFooBarRoute + '/api/parsed-params/$id/$childId': typeof ApiParsedParamsIdChildIdRoute } export interface FileRouteTypes { fileRoutesByFullPath: FileRoutesByFullPath @@ -139,7 +158,9 @@ export interface FileRouteTypes { | '/methods/only-any' | '/methods/' | '/api/params/$foo' + | '/api/parsed-params/$id' | '/api/params/$foo/$bar' + | '/api/parsed-params/$id/$childId' fileRoutesByTo: FileRoutesByTo to: | '/' @@ -152,7 +173,9 @@ export interface FileRouteTypes { | '/methods/only-any' | '/methods' | '/api/params/$foo' + | '/api/parsed-params/$id' | '/api/params/$foo/$bar' + | '/api/parsed-params/$id/$childId' id: | '__root__' | '/' @@ -166,7 +189,9 @@ export interface FileRouteTypes { | '/methods/only-any' | '/methods/' | '/api/params/$foo' + | '/api/parsed-params/$id' | '/api/params/$foo/$bar' + | '/api/parsed-params/$id/$childId' fileRoutesById: FileRoutesById } export interface RootRouteChildren { @@ -179,6 +204,7 @@ export interface RootRouteChildren { ApiMiddlewareContextRoute: typeof ApiMiddlewareContextRoute ApiOnlyAnyRoute: typeof ApiOnlyAnyRoute ApiParamsFooRouteRoute: typeof ApiParamsFooRouteRouteWithChildren + ApiParsedParamsIdRoute: typeof ApiParsedParamsIdRouteWithChildren } declare module '@tanstack/react-router' { @@ -260,6 +286,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof ApiParamsFooRouteRouteImport parentRoute: typeof rootRouteImport } + '/api/parsed-params/$id': { + id: '/api/parsed-params/$id' + path: '/api/parsed-params/$id' + fullPath: '/api/parsed-params/$id' + preLoaderRoute: typeof ApiParsedParamsIdRouteImport + parentRoute: typeof rootRouteImport + } '/api/params/$foo/$bar': { id: '/api/params/$foo/$bar' path: '/$bar' @@ -267,6 +300,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof ApiParamsFooBarRouteImport parentRoute: typeof ApiParamsFooRouteRoute } + '/api/parsed-params/$id/$childId': { + id: '/api/parsed-params/$id/$childId' + path: '/$childId' + fullPath: '/api/parsed-params/$id/$childId' + preLoaderRoute: typeof ApiParsedParamsIdChildIdRouteImport + parentRoute: typeof ApiParsedParamsIdRoute + } } } @@ -295,6 +335,17 @@ const ApiParamsFooRouteRouteChildren: ApiParamsFooRouteRouteChildren = { const ApiParamsFooRouteRouteWithChildren = ApiParamsFooRouteRoute._addFileChildren(ApiParamsFooRouteRouteChildren) +interface ApiParsedParamsIdRouteChildren { + ApiParsedParamsIdChildIdRoute: typeof ApiParsedParamsIdChildIdRoute +} + +const ApiParsedParamsIdRouteChildren: ApiParsedParamsIdRouteChildren = { + ApiParsedParamsIdChildIdRoute: ApiParsedParamsIdChildIdRoute, +} + +const ApiParsedParamsIdRouteWithChildren = + ApiParsedParamsIdRoute._addFileChildren(ApiParsedParamsIdRouteChildren) + const rootRouteChildren: RootRouteChildren = { IndexRoute: IndexRoute, MethodsRouteRoute: MethodsRouteRouteWithChildren, @@ -305,6 +356,7 @@ const rootRouteChildren: RootRouteChildren = { ApiMiddlewareContextRoute: ApiMiddlewareContextRoute, ApiOnlyAnyRoute: ApiOnlyAnyRoute, ApiParamsFooRouteRoute: ApiParamsFooRouteRouteWithChildren, + ApiParsedParamsIdRoute: ApiParsedParamsIdRouteWithChildren, } export const routeTree = rootRouteImport ._addFileChildren(rootRouteChildren) diff --git a/e2e/react-start/server-routes/src/routes/api/parsed-params.$id.$childId.tsx b/e2e/react-start/server-routes/src/routes/api/parsed-params.$id.$childId.tsx new file mode 100644 index 00000000000..06697a22cc5 --- /dev/null +++ b/e2e/react-start/server-routes/src/routes/api/parsed-params.$id.$childId.tsx @@ -0,0 +1,51 @@ +import { createFileRoute } from '@tanstack/react-router' +import { createMiddleware } from '@tanstack/react-start' + +const handleInvalidParams = createMiddleware().server(async ({ next }) => { + try { + return await next() + } catch (error) { + if (error instanceof Error && error.message === 'Invalid child id') { + return new Response(error.message, { status: 400 }) + } + throw error + } +}) + +export const Route = createFileRoute('/api/parsed-params/$id/$childId')({ + params: { + parse: (params) => { + const childId = Number(params.childId) + if (!Number.isFinite(childId)) { + throw new Error('Invalid child id') + } + return { childId } + }, + stringify: (params) => ({ childId: String(params.childId) }), + }, + server: { + handlers: ({ createHandlers }) => + createHandlers({ + GET: { + middleware: [handleInvalidParams], + handler: ({ params, request, next }) => { + if (new URL(request.url).searchParams.has('render')) { + return next({ context: { handlerParams: params } }) + } + const id: number = params.id + const childId: number = params.childId + return Response.json({ id, childId }) + }, + }, + }), + }, + loader: ({ params, serverContext }) => ({ + loaderParams: params, + handlerParams: serverContext?.handlerParams, + }), + component: () => ( +
+      {JSON.stringify(Route.useLoaderData())}
+    
+ ), +}) diff --git a/e2e/react-start/server-routes/src/routes/api/parsed-params.$id.ts b/e2e/react-start/server-routes/src/routes/api/parsed-params.$id.ts new file mode 100644 index 00000000000..0e5f0ab8a94 --- /dev/null +++ b/e2e/react-start/server-routes/src/routes/api/parsed-params.$id.ts @@ -0,0 +1,16 @@ +import { createFileRoute } from '@tanstack/react-router' + +export const Route = createFileRoute('/api/parsed-params/$id')({ + params: { + parse: (params) => ({ id: Number(params.id) }), + stringify: (params) => ({ id: String(params.id) }), + }, + server: { + handlers: { + GET: ({ params }) => { + const id: number = params.id + return Response.json({ id }) + }, + }, + }, +}) diff --git a/e2e/react-start/server-routes/tests/server-routes.spec.ts b/e2e/react-start/server-routes/tests/server-routes.spec.ts index 97249fd733e..0ede9eaac85 100644 --- a/e2e/react-start/server-routes/tests/server-routes.spec.ts +++ b/e2e/react-start/server-routes/tests/server-routes.spec.ts @@ -1,6 +1,46 @@ import { expect } from '@playwright/test' import { test } from '@tanstack/router-e2e-utils' +test('server handlers receive parsed path params (issue #8431)', async ({ + request, +}) => { + const response = await request.get('/api/parsed-params/44') + + expect(response.status()).toBe(200) + expect(await response.json()).toEqual({ id: 44 }) +}) + +test('createHandlers receives parsed parent and child path params', async ({ + request, +}) => { + const response = await request.get('/api/parsed-params/44/2') + + expect(response.status()).toBe(200) + expect(await response.json()).toEqual({ id: 44, childId: 2 }) +}) + +test('handler middleware can catch path parameter parsing errors', async ({ + request, +}) => { + const response = await request.get('/api/parsed-params/44/invalid') + + expect(response.status()).toBe(400) + expect(await response.text()).toBe('Invalid child id') +}) + +test('parsed path params agree when a server handler defers to rendering', async ({ + page, +}) => { + await page.goto('/api/parsed-params/44/2?render') + + const result = page.getByTestId('parsed-params') + await expect(result).toBeVisible() + expect(JSON.parse((await result.textContent())!)).toEqual({ + loaderParams: { id: 44, childId: 2 }, + handlerParams: { id: 44, childId: 2 }, + }) +}) + test('merge-middleware-context', async ({ page }) => { await page.goto('/merge-middleware-context') diff --git a/packages/start-server-core/src/createStartHandler.ts b/packages/start-server-core/src/createStartHandler.ts index 84af2d4558f..4c3ba81ecaf 100644 --- a/packages/start-server-core/src/createStartHandler.ts +++ b/packages/start-server-core/src/createStartHandler.ts @@ -45,6 +45,7 @@ import type { AnyRequestMiddleware, AnyStartInstanceOptions, RouteMethod, + RouteMethodHandlerFn, RouterEntry, StartEntry, } from '@tanstack/start-client-core' @@ -906,6 +907,32 @@ async function handleRedirectResponse( return ssrResponse } +function withParsedParams( + handler: RouteMethodHandlerFn, + matchedRoutes: ReadonlyArray, +): TODO { + if ( + !matchedRoutes.some( + (route) => route.options.params?.parse ?? route.options.parseParams, + ) + ) { + return handler + } + + return (ctx: TODO) => { + // Parse inside the pipeline so middleware can catch errors. Keep the raw + // params for app-router validation when the handler defers to rendering. + const params = Object.assign(Object.create(null), ctx.params) + for (const route of matchedRoutes) { + const parse = route.options.params?.parse ?? route.options.parseParams + if (parse) { + Object.assign(params, parse(params)) + } + } + return handler({ ...ctx, params }) + } +} + async function handleServerRoutes({ getRouter, request, @@ -975,27 +1002,23 @@ async function handleServerRoutes({ if (handler) { const mayDefer = !!foundRoute.options.component - if (typeof handler === 'function') { - if (!mayDefer) { - terminalHandler = handler - terminalNext = throwIfMayNotDefer - } else { - routeMiddlewares.push(handler) - } - } else { + if (typeof handler !== 'function') { if (handler.middleware?.length) { const handlerMiddlewares = flattenMiddlewares(handler.middleware) for (const m of handlerMiddlewares) { routeMiddlewares.push(m.options.server) } } - if (handler.handler) { - if (!mayDefer) { - terminalHandler = handler.handler - terminalNext = throwIfMayNotDefer - } else { - routeMiddlewares.push(handler.handler) - } + } + const routeHandler = + typeof handler === 'function' ? handler : handler.handler + if (routeHandler) { + const parsedHandler = withParsedParams(routeHandler, matchedRoutes) + if (!mayDefer) { + terminalHandler = parsedHandler + terminalNext = throwIfMayNotDefer + } else { + routeMiddlewares.push(parsedHandler) } } } diff --git a/packages/start-server-core/tests/createStartHandler.test.ts b/packages/start-server-core/tests/createStartHandler.test.ts index 18bbbcb9f28..8044b6b52f1 100644 --- a/packages/start-server-core/tests/createStartHandler.test.ts +++ b/packages/start-server-core/tests/createStartHandler.test.ts @@ -1626,6 +1626,99 @@ describe('createStartHandler direct server routes', () => { expect(render).not.toHaveBeenCalled() }) + it.each( + [false, true].flatMap((component) => + ['function', 'object'].flatMap((handlerKind) => + ['params.parse', 'parseParams'].map((parserKind) => ({ + component, + handlerKind, + parserKind, + })), + ), + ), + )( + 'parses server handler params (component=$component, handler=$handlerKind, parser=$parserKind)', + async ({ component, handlerKind, parserKind }) => { + const parse = (params: { itemId: string }) => ({ + itemId: Number(params.itemId), + }) + const routeHandler = vi.fn(({ params }: any) => Response.json(params)) + startMocks.router = makeRouter({ + path: '/items/$itemId', + component: component ? () => null : undefined, + ...(parserKind === 'params.parse' + ? { params: { parse } } + : { parseParams: parse }), + server: { + handlers: { + GET: + handlerKind === 'function' + ? routeHandler + : { handler: routeHandler }, + }, + }, + }) + const render = vi.fn(() => new Response('must not render')) + const handler = createStartHandler(render) + + const response = await handler( + new Request('http://localhost/items/42'), + {}, + ) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ itemId: 42 }) + expect(routeHandler).toHaveBeenCalledOnce() + expect(render).not.toHaveBeenCalled() + }, + ) + + it.each(['route', 'handler'] as const)( + 'lets %s middleware catch terminal handler parameter parsing errors', + async (placement) => { + const error = new Error('Invalid item id') + const middleware = createMiddleware().server(async ({ next }) => { + try { + return await next() + } catch (caught) { + expect(caught).toBe(error) + return new Response(error.message, { status: 400 }) + } + }) + const routeHandler = vi.fn(() => new Response('must not run')) + startMocks.router = makeRouter({ + path: '/items/$itemId', + component: undefined, + params: { + parse: () => { + throw error + }, + }, + server: { + ...(placement === 'route' ? { middleware: [middleware] } : {}), + handlers: { + GET: + placement === 'handler' + ? { middleware: [middleware], handler: routeHandler } + : routeHandler, + }, + }, + }) + const render = vi.fn(() => new Response('must not render')) + const handler = createStartHandler(render) + + const response = await handler( + new Request('http://localhost/items/nope'), + {}, + ) + + expect(response.status).toBe(400) + await expect(response.text()).resolves.toBe('Invalid item id') + expect(routeHandler).not.toHaveBeenCalled() + expect(render).not.toHaveBeenCalled() + }, + ) + it.each(['route', 'handler'] as const)( 'runs %s middleware instead of incorrectly taking the direct path', async (placement) => {