diff --git a/.changeset/middleware-validator-type-cycle.md b/.changeset/middleware-validator-type-cycle.md new file mode 100644 index 00000000..9218aadc --- /dev/null +++ b/.changeset/middleware-validator-type-cycle.md @@ -0,0 +1,5 @@ +--- +"@marko/run": patch +--- + +Fix route middleware and handler types collapsing to `any` when a middleware derives `next()` data from its context while a downstream handler declares validators. diff --git a/agent-feedback/dx.md b/agent-feedback/dx.md index 5b7304f2..852912d4 100644 --- a/agent-feedback/dx.md +++ b/agent-feedback/dx.md @@ -209,9 +209,3 @@ CodeRabbit's pre-merge checks (configured in the organization UI; the repo has n `cspell.json` › `words` | 2026-08-11 | impact:med | effort:low `pnpm run lint` fails on a clean `main` with a single cspell error: `agent-feedback/cleanup.md` uses "macrotask", which is not in `cspell.json`'s `words` list. The word arrived with 445114c (#256) and the check covers `**/*.{md,ts,marko}`, so every branch cut from main inherits a red lint until the word is added. Re-verify: `npx cspell "**/*.{md,ts,marko}" --no-progress` on main reports one issue. - -## A `params` validator on a route with two `$` segments loses its typing - -`packages/run/src/runtime/types.ts` › `RouteForFileDef` | 2026-08-17 | impact:low | effort:med - -In an app with `api/workspaces/$id/+middleware.ts` (a `Run.ALL` middleware calling `next({ workspace })`) and a leaf `api/workspaces/$id/review/$commentId/+handler.ts` exporting `Run.DELETE({ params({ id, commentId }) { … } }, (ctx) => …)`, `tsc` reports TS7031 (`id` / `commentId` implicitly any) on the validator's destructured argument and TS2554 ("Expected 1 arguments, but got 2") on the `Run.DELETE` call, so the two-argument form is unusable there. The identical shape on a one-segment route with no parent middleware (`api/procs/$pid/+handler.ts`, `Run.DELETE({ params({ pid }) … }, handler)`) type-checks. Something in the merge of the parent middleware's options with the leaf's `params` (`MergedRouteOptionsForFile` → `RouteForFileDef.params`) collapses when the path has two params, since the workaround is simply parsing `ctx.params.commentId` inside the handler. Repro: the two files above in a fixture under `packages/run/src/__tests__/fixtures/`, then `tsc` on it; a passing fixture with the validator form is the fix's test. diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/.marko-run/routes.d.ts b/packages/run/src/__tests__/fixtures/params-validator-types/.marko-run/routes.d.ts new file mode 100644 index 00000000..b917c161 --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/.marko-run/routes.d.ts @@ -0,0 +1,87 @@ +/* + WARNING: This file is automatically generated and any changes made to it will be overwritten without warning. + Do NOT manually edit this file or your changes will be lost. +*/ + +import { NotHandled, NotMatched, GetPaths, PostPaths, GetablePath, GetableHref, PostablePath, PostableHref, Platform } from "@marko/run/namespace"; +import type * as $ from "@marko/run"; + + +declare module "@marko/run" { + interface App extends $.DefineRoutes<{ + "/api/procs/$pid": [H1]; + "/api/workspaces/$id/review/$commentId": [M1, H2]; + }> {} +} + +type M1 = $.Middleware<"M1", typeof import("../src/routes/api/workspaces/$id/+middleware")>; +declare module "../src/routes/api/workspaces/$id/+middleware" { + const Run: $.Namespace; + namespace Run { + type Context = $.ContextForFile; + } + + /** @deprecated use `Run` namespace instead */ + namespace MarkoRun { + export { NotHandled, NotMatched, GetPaths, PostPaths, GetablePath, GetableHref, PostablePath, PostableHref, Platform }; + export type Route = $.Routes["/api/workspaces/$id/review/$commentId"]; + export type Context = $.MultiRouteContext; + export type Handler = $.HandlerLike; + export type GET = $.HandlerLike; + export type HEAD = $.HandlerLike; + export type POST = $.HandlerLike; + export type PUT = $.HandlerLike; + export type DELETE = $.HandlerLike; + export type PATCH = $.HandlerLike; + export type OPTIONS = $.HandlerLike; + export type QUERY = $.HandlerLike; + } +} + +type H1 = $.Handler<"H1", typeof import("../src/routes/api/procs/$pid/+handler")>; +declare module "../src/routes/api/procs/$pid/+handler" { + const Run: $.Namespace

; + namespace Run { + type Context = $.ContextForFile

; + } + + /** @deprecated use `Run` namespace instead */ + namespace MarkoRun { + export { NotHandled, NotMatched, GetPaths, PostPaths, GetablePath, GetableHref, PostablePath, PostableHref, Platform }; + export type Route = $.Routes["/api/procs/$pid"]; + export type Context = $.MultiRouteContext; + export type Handler = $.HandlerLike; + export type GET = $.HandlerLike; + export type HEAD = $.HandlerLike; + export type POST = $.HandlerLike; + export type PUT = $.HandlerLike; + export type DELETE = $.HandlerLike; + export type PATCH = $.HandlerLike; + export type OPTIONS = $.HandlerLike; + export type QUERY = $.HandlerLike; + } +} + +type H2 = $.Handler<"H2", typeof import("../src/routes/api/workspaces/$id/review/$commentId/+handler")>; +declare module "../src/routes/api/workspaces/$id/review/$commentId/+handler" { + const Run: $.Namespace

; + namespace Run { + type Context = $.ContextForFile

; + } + + /** @deprecated use `Run` namespace instead */ + namespace MarkoRun { + export { NotHandled, NotMatched, GetPaths, PostPaths, GetablePath, GetableHref, PostablePath, PostableHref, Platform }; + export type Route = $.Routes["/api/workspaces/$id/review/$commentId"]; + export type Context = $.MultiRouteContext; + export type Handler = $.HandlerLike; + export type GET = $.HandlerLike; + export type HEAD = $.HandlerLike; + export type POST = $.HandlerLike; + export type PUT = $.HandlerLike; + export type DELETE = $.HandlerLike; + export type PATCH = $.HandlerLike; + export type OPTIONS = $.HandlerLike; + export type QUERY = $.HandlerLike; + } +} diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/__snapshots__/dev.expected.md b/packages/run/src/__tests__/fixtures/params-validator-types/__snapshots__/dev.expected.md new file mode 100644 index 00000000..df97ae65 --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/__snapshots__/dev.expected.md @@ -0,0 +1,5 @@ +# Loading + +# Step 0 +ctx=>deleteComment(ctx) + diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/__snapshots__/preview.expected.md b/packages/run/src/__tests__/fixtures/params-validator-types/__snapshots__/preview.expected.md new file mode 100644 index 00000000..df97ae65 --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/__snapshots__/preview.expected.md @@ -0,0 +1,5 @@ +# Loading + +# Step 0 +ctx=>deleteComment(ctx) + diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/procs/$pid/+handler.ts b/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/procs/$pid/+handler.ts new file mode 100644 index 00000000..cd14bed2 --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/procs/$pid/+handler.ts @@ -0,0 +1,21 @@ +export const DELETE = Run.DELETE( + { + params({ pid }) { + return { pid }; + }, + }, + (ctx) => { + return Response.json({ pid: ctx.params.pid }); + }, +); + +// No middleware runs upstream, so this file's handlers get return checking. +export const validReturns = [ + Run.GET(() => new Response("ok")), + Run.GET(async () => Response.json({ ok: true })), + Run.GET((ctx, next) => next()), +]; +// @ts-expect-error a handler cannot return a bare number +export const badNumber = Run.GET(() => 42); +// @ts-expect-error a handler cannot return a bare object in place of a Response +export const badObject = Run.GET(() => ({ plain: "object" })); diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/workspaces/$id/+middleware.ts b/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/workspaces/$id/+middleware.ts new file mode 100644 index 00000000..6dbe77bb --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/workspaces/$id/+middleware.ts @@ -0,0 +1,16 @@ +export default Run.ALL( + { + search({ limit }) { + return { limit: limit ? Number(limit) : 0 }; + }, + }, + (ctx, next) => { + if ("commentId" in ctx.params) { + const merged: number = ctx.params.commentId; + // @ts-expect-error the DELETE validator types commentId as number + const bad: boolean = ctx.params.commentId; + void merged, void bad; + } + return next({ workspace: `ws-${ctx.params.id}` }); + }, +); diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/workspaces/$id/review/$commentId/+handler.ts b/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/workspaces/$id/review/$commentId/+handler.ts new file mode 100644 index 00000000..7ee11261 --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/src/routes/api/workspaces/$id/review/$commentId/+handler.ts @@ -0,0 +1,37 @@ +import type { Context, GetContext } from "@marko/run"; + +export const DELETE = Run.DELETE( + { + params({ id, commentId }) { + return { id, commentId: Number(commentId) }; + }, + }, + (ctx) => { + const asPlainContext: Context = ctx; + const asScopedContext: GetContext<"/api/workspaces/$id/review/$commentId"> = + ctx; + // @ts-expect-error the deprecated MarkoRun.Context types params as raw + // strings, so a context whose validator produced a number never fit it + const asLegacyContext: MarkoRun.Context = ctx; + void asPlainContext, void asScopedContext, void asLegacyContext; + const validated: number = ctx.params.commentId; + const upstream: string = ctx.data.workspace; + const fromMiddleware: number = ctx.search.limit; + // @ts-expect-error the validator types commentId as number + const badParam: boolean = ctx.params.commentId; + // @ts-expect-error the middleware data types workspace as string + const badData: boolean = ctx.data.workspace; + // @ts-expect-error the middleware search validator types limit as number + const badSearch: boolean = ctx.search.limit; + void validated, void upstream, void fromMiddleware, void badParam; + void badData, void badSearch; + return Response.json({ + id: ctx.params.id, + commentId: ctx.params.commentId, + }); + }, +); + +export const routeLevel: number = null as unknown as Run.Context["params"]["commentId"]; +// @ts-expect-error route-level params keep the validated number type +export const routeLevelBad: boolean = null as unknown as Run.Context["params"]["commentId"]; diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/test.config.ts b/packages/run/src/__tests__/fixtures/params-validator-types/test.config.ts new file mode 100644 index 00000000..7951a292 --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/test.config.ts @@ -0,0 +1,15 @@ +import assert from "assert"; + +import type { Step, StepContext } from "../../main.test"; + +export const path = "/api/procs/42"; + +async function deleteComment({ page }: StepContext) { + const res = await page.fetch( + new URL("/api/workspaces/w1/review/7", page.url()).href, + { method: "DELETE" }, + ); + assert.deepEqual(await res.json(), { id: "w1", commentId: 7 }); +} + +export const steps: Step[] = [(ctx) => deleteComment(ctx)]; diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/tsconfig.check.json b/packages/run/src/__tests__/fixtures/params-validator-types/tsconfig.check.json new file mode 100644 index 00000000..be7e4d43 --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/tsconfig.check.json @@ -0,0 +1,11 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": true, + "baseUrl": ".", + "paths": { + "@marko/run": ["../../../runtime/index.ts"], + "@marko/run/namespace": ["../../../runtime/namespace.ts"] + } + } +} diff --git a/packages/run/src/__tests__/fixtures/params-validator-types/tsconfig.json b/packages/run/src/__tests__/fixtures/params-validator-types/tsconfig.json new file mode 100644 index 00000000..03ff88a5 --- /dev/null +++ b/packages/run/src/__tests__/fixtures/params-validator-types/tsconfig.json @@ -0,0 +1,4 @@ +{ + "extends": "../../tsconfig-base.json", + "include": ["src/**/*", ".marko-run/*"], +} \ No newline at end of file diff --git a/packages/run/src/__tests__/typecheck.test.ts b/packages/run/src/__tests__/typecheck.test.ts index 41d46982..61658a1f 100644 --- a/packages/run/src/__tests__/typecheck.test.ts +++ b/packages/run/src/__tests__/typecheck.test.ts @@ -57,3 +57,37 @@ describe("test sources type-check", () => { ); }); }); + +describe("route type generation type-checks", () => { + // The fixture's middleware and handler each need the other's inferred + // module type — the shape that collapses both to `any` (TS7022). Its + // `@ts-expect-error` probes make silent widening fail this compile too. + it("keeps middleware and validator types across the module cycle", function () { + this.timeout(120000); + const result = spawnSync( + process.execPath, + [ + require.resolve("typescript/lib/tsc.js"), + "-p", + path.join( + __dirname, + "fixtures", + "params-validator-types", + "tsconfig.check.json", + ), + "--pretty", + "false", + ], + { encoding: "utf-8", timeout: 90000 }, + ); + + const localErrors = (result.stdout || "") + .split("\n") + // The compile pulls in the whole workspace graph, which carries known + // declaration-level noise; only errors in the fixture's sources are + // the reproduction. + .filter((line) => /params-validator-types[/\\]src[/\\]/.test(line)); + + assert.deepEqual(localErrors, []); + }); +}); diff --git a/packages/run/src/runtime/index.ts b/packages/run/src/runtime/index.ts index 3cfe955b..55baf1b7 100644 --- a/packages/run/src/runtime/index.ts +++ b/packages/run/src/runtime/index.ts @@ -77,10 +77,15 @@ export type { export type { App, Context, + ContextBase, ContextForFile, + DefinedHandler, + DefinedHandlerTypes, + DefineHandler, DefineRoutes, Empty, Fetch, + FileContext, GetContext, Handler, HandlerTypes, diff --git a/packages/run/src/runtime/types.ts b/packages/run/src/runtime/types.ts index 9a77f580..d36a288f 100644 --- a/packages/run/src/runtime/types.ts +++ b/packages/run/src/runtime/types.ts @@ -472,6 +472,12 @@ type ComposedHandlerData = MergeTuple<{ [K in keyof Handlers]: HandlerValueData; }>; type Exact = T & Record, never>; +/** + * Bars a function from matching the all-optional options parameter — every + * function has `apply` — so a mistyped handler argument fails the overload + * instead of being accepted as an options object. + */ +type NotAFunction = { apply?: never }; type DefineHandlerOptions = [Verb] extends [ HttpVerbWithoutBody, ] @@ -555,6 +561,27 @@ export interface RouteForFileDef< ? T : Record; } +/** + * A file's view of one route context. Deliberately not `Context<...>`: its + * constraint check forces every member — including `data`, and with it + * another file's possibly mid-resolution export. A standalone interface + * resolves each member only when read, after the exports it needs can + * resolve body-free. + */ +export interface FileContext< + F extends File, + Path extends keyof AppPaths, + Verb extends HttpVerb, + Options, +> extends ContextBase { + readonly route: Path; + readonly method: Verb; + readonly meta: RouteForFileDef["meta"]; + readonly params: RouteForFileDef["params"]; + readonly search: RouteForFileDef["search"]; + readonly body: RouteForFileDef["body"]; + readonly data: RouteForFileDef["data"]; +} type ContextForFileWithOptions< F extends File, Verb extends HttpVerbOrAll, @@ -562,105 +589,141 @@ type ContextForFileWithOptions< > = Union<{ [Path in PathsForFile]: Union<{ [V in VerbsForPath]: V extends HttpVerb - ? Context> + ? FileContext : never; }>; }>; -export type DefineHandler = { - ( - handlers: HandlerArray< - ContextForFileWithOptions & {}, +type FileHandlerOptions< + F extends File, + Verb extends HttpVerbOrAll, +> = DefineHandlerOptions>; +type OptionsArg = Exact< + Options, + FileHandlerOptions +> & + NotAFunction; +type DefinedHandlerFunction = ( + ctx: Ctx, + next: NextFunction, +) => Return; +type DefinedHandlerArray = readonly ( + | { + [__run__.TYPES]: { + options: any; + data: any; + }; + } + | DefinedHandlerFunction +)[]; +/** + * Spelled as an object literal: a deferred conditional stays lazy as a + * structural member but is forced as a direct type argument to a named + * generic like HandlerTypes, which would type the handler body while another + * file's export is still resolving. + */ +export type DefinedHandlerTypes< + F extends File, + Verb extends HttpVerbOrAll, + Options, + Handlers extends readonly unknown[], +> = { + context: ContextForFileWithOptions; + verb: Verb; + options: Options; + data: ComposedHandlerData extends infer Data + ? [Data] extends [never] + ? Empty + : Data extends Record + ? Data + : Empty + : never; + passthrough: [ComposedHandlerData] extends [never] ? false : true; +}; +export type DefinedHandler< + F extends File, + Verb extends HttpVerbOrAll, + Options, + Handlers extends readonly unknown[], +> = Typed< + NormalizedHandlerFunction, + DefinedHandlerTypes +>; +/** + * True when middleware shares any of the file's routes, so resolving its + * export while another is mid-flight is possible. Namespace resolves this — + * not a `Return` default — because the deferred form needs the literal `any` + * type for the checker to skip return comparison. + */ +type FileHasUpstream = + true extends Union<{ + [P in PathsForFile]: AppPaths[P]["files"]["middleware"] extends [] + ? false + : true; + }> + ? true + : false; +/** + * Captures whole function types rather than their returns: a function type's + * return resolves lazily and an `any` return target never asks for it, so + * other files can read this export's merged options without its body ever + * being typed. Body-derived data reaches the export as conditionals typed + * only when something reads them. + * + * `Return` is the callbacks' return target: `any` keeps the export + * resolvable body-free, required wherever another route file's inference can + * entangle this one; `HandlerReturn` checks the return position, safe only + * when nothing else can be mid-resolution. + */ +export type DefineHandler< + F extends File, + Verb extends HttpVerbOrAll, + Return = any, +> = { + ( + handlers: DefinedHandlerArray< + ContextForFileWithOptions, Return > & Handlers, - ): Typed< - NormalizedHandlerFunction>, - HandlerTypes< - ContextForFileWithOptions & {}, - Verb, - ComposedHandlerOptions, - ComposedHandlerData - > - >; + ): DefinedHandler, Handlers>; < - const Options extends DefineHandlerOptions< - Verb, - ContextForFileWithOptions & {} - >, + const Options extends FileHandlerOptions, const Handlers extends readonly unknown[], - Return extends unknown[], >( - options: Exact< - Options, - DefineHandlerOptions & {}> - >, - handlers: HandlerArray< - ContextForFileWithOptions & {}, + options: OptionsArg, + handlers: DefinedHandlerArray< + ContextForFileWithOptions, Return > & Handlers, - ): Typed< - NormalizedHandlerFunction< - Verb, - MergeHandlerOptions, Options> - >, - HandlerTypes< - ContextForFileWithOptions & {}, - Verb, - MergeHandlerOptions, Options>, - ComposedHandlerData - > - >; - ( - handler: HandlerFunction< - ContextForFileWithOptions & {}, - Return - >, - ): NormalizedHandler< - ContextForFileWithOptions & {}, + ): DefinedHandler< + F, Verb, - Return, - {} + MergeHandlerOptions, Options>, + Handlers >; < - const Options extends DefineHandlerOptions< - Verb, - ContextForFileWithOptions & {} + H extends DefinedHandlerFunction< + ContextForFileWithOptions, + Return >, >( - options: Exact< - Options, - DefineHandlerOptions & {}> - >, - ): NormalizedHandler< - ContextForFileWithOptions & {}, - Verb, - {}, - Options - >; + handler: H, + ): DefinedHandler; + >( + options: OptionsArg, + ): DefinedHandler; < - const Options extends DefineHandlerOptions< - Verb, - ContextForFileWithOptions & {} - >, - Return, - >( - options: Exact< - Options, - DefineHandlerOptions & {}> - >, - handler: HandlerFunction< - NoInfer> & {}, + const Options extends FileHandlerOptions, + H extends DefinedHandlerFunction< + NoInfer>, Return >, - ): NormalizedHandler< - ContextForFileWithOptions & {}, - Verb, - Return, - Options - >; + >( + options: OptionsArg, + handler: H, + ): DefinedHandler; }; - export type GlobalDefineHandler = { ( handlers: HandlerArray & Handlers, @@ -943,7 +1006,9 @@ export type Namespace = Typed< } : F["type"] extends "handler" ? { - [Verb in HttpVerb]: DefineHandler; + [Verb in HttpVerb]: FileHasUpstream extends true + ? DefineHandler + : DefineHandler; } : Empty) & NamespaceVerb, @@ -973,14 +1038,8 @@ export type DefineRoutes = { >; }; export interface Platform {} -export interface Context { - readonly route: T["path"]; - readonly method: T["method"]; - readonly meta: T["meta"]; - readonly params: T["params"]; - readonly search: T["search"]; - readonly body: T["body"]; - readonly data: T["data"]; +/** The route-independent half of a context, shared with FileContext. */ +export interface ContextBase { readonly url: URL; readonly request: Request; readonly platform: Platform; @@ -998,6 +1057,15 @@ export interface Context { redirect(to: string | URL, status?: number): Response; back(fallback?: string | URL, status?: number): Response; } +export interface Context extends ContextBase { + readonly route: T["path"]; + readonly method: T["method"]; + readonly meta: T["meta"]; + readonly params: T["params"]; + readonly search: T["search"]; + readonly body: T["body"]; + readonly data: T["data"]; +} export type GetContext< Scope extends keyof AppPaths | `*` | `/${string}*` | object = "*", Verb extends