-
Notifications
You must be signed in to change notification settings - Fork 364
feat(context): Added typed context to h3 #1379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import type { H3EventContext } from "./context.ts"; | ||
| import type { HTTPHandler, EventHandler, Middleware } from "./handler.ts"; | ||
| import type { HTTPHandler, EventHandler, Middleware, EventHandlerRequest } from "./handler.ts"; | ||
| import type { HTTPError } from "../error.ts"; | ||
| import type { MaybePromise } from "./_utils.ts"; | ||
| import type { FetchHandler, ServerRequest } from "srvx"; | ||
|
|
@@ -60,8 +60,8 @@ export interface H3Route { | |
|
|
||
| // --- H3 App --- | ||
|
|
||
| export type RouteOptions = { | ||
| middleware?: Middleware[]; | ||
| export type RouteOptions<_ContextT extends H3EventContext = H3EventContext> = { | ||
| middleware?: Middleware<_ContextT>[]; | ||
| meta?: H3RouteMeta; | ||
| }; | ||
|
|
||
|
|
@@ -114,7 +114,7 @@ export declare class H3Core { | |
| "~addRoute"(_route: H3Route): void; | ||
| } | ||
|
|
||
| export declare class H3 extends H3Core { | ||
| export declare class H3<_ContextT extends H3EventContext = H3EventContext> extends H3Core { | ||
| /** @internal */ | ||
| "~rou3": RouterContext; | ||
|
|
||
|
|
@@ -128,22 +128,36 @@ export declare class H3 extends H3Core { | |
| request( | ||
| request: ServerRequest | URL | string, | ||
| options?: RequestInit, | ||
| context?: H3EventContext, | ||
| context?: _ContextT, | ||
| ): Response | Promise<Response>; | ||
|
|
||
| /** | ||
| * Register a global middleware. | ||
| */ | ||
| use(route: string, handler: Middleware | H3, opts?: MiddlewareOptions): this; | ||
| use(handler: Middleware | H3, opts?: MiddlewareOptions): this; | ||
| use( | ||
| route: string, | ||
| handler: Middleware<_ContextT> | H3<_ContextT>, | ||
| opts?: MiddlewareOptions, | ||
| ): this; | ||
| use(handler: Middleware<_ContextT> | H3<_ContextT>, opts?: MiddlewareOptions): this; | ||
|
|
||
| /** | ||
| * Extend the event context with a key and a typed value | ||
| * @returns a new H3 instance with the extended context type | ||
| */ | ||
| extendContext<K extends string, V>( | ||
| key: K, | ||
| valFn: (event: H3Event<EventHandlerRequest, _ContextT>) => V, | ||
| ): H3<_ContextT & Record<K, V>>; | ||
| extendContext<K extends string, V>(key: K, val: V): H3<_ContextT & Record<K, V>>; | ||
|
|
||
| /** | ||
| * Register a route handler for the specified HTTP method and route. | ||
| */ | ||
| on( | ||
| method: HTTPMethod | Lowercase<HTTPMethod> | "", | ||
| route: string, | ||
| handler: HTTPHandler, | ||
| handler: HTTPHandler<_ContextT>, | ||
| opts?: RouteOptions, | ||
|
Comment on lines
157
to
161
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Show the route options definition and the updated route-registration signatures.
sed -n '63,66p' src/types/h3.ts
sed -n '153,190p' src/types/h3.tsRepository: h3js/h3 Length of output: 1835 🏁 Script executed: #!/bin/bash
# Find the Middleware type definition and related context types
rg "export type Middleware" src/types/ -A 3
rg "export type HTTPHandler" src/types/ -A 3
rg "export.*H3EventContext" src/types/ -A 2Repository: h3js/h3 Length of output: 1638 🏁 Script executed: #!/bin/bash
# Check all usages of RouteOptions and how it's threaded through the codebase
rg "RouteOptions" src/types/ -B 2 -A 1
rg "RouteOptions" src/ --include="*.ts" | grep -v test | head -30Repository: h3js/h3 Length of output: 1936 🏁 Script executed: #!/bin/bash
# Check H3Route definition and implementation details
rg "export.*type H3Route" src/types/ -A 5
rg "class H3" src/ -t ts -A 10 | head -40Repository: h3js/h3 Length of output: 1462 🏁 Script executed: #!/bin/bash
# Look for H3Route type definition
rg "type H3Route" src/types/ -B 2 -A 5Repository: h3js/h3 Length of output: 33 🏁 Script executed: #!/bin/bash
# Search for H3Route definition more broadly
rg "H3Route" src/types/ -B 1 -A 3
rg "interface H3Route" src/ -t ts -A 5Repository: h3js/h3 Length of output: 2954 🏁 Script executed: #!/bin/bash
# Check EventHandler definition
rg "export type EventHandler" src/types/ -A 3
# Check the actual implementation of on() method
sed -n '100,250p' src/h3.ts | head -80Repository: h3js/h3 Length of output: 3357 🏁 Script executed: #!/bin/bash
# Look at more of the on() implementation and check all verb methods
sed -n '100,160p' src/h3.ts | cat -n
# Also check what HTTPHandler signature expects
rg "export type HTTPHandler" src/types/handler.ts -A 5Repository: h3js/h3 Length of output: 2646 Route-level middleware loses the typed context.
Make Suggested direction-export type RouteOptions = {
- middleware?: Middleware[];
+export type RouteOptions<_ContextT extends H3EventContext = H3EventContext> = {
+ middleware?: Middleware<_ContextT>[];
meta?: H3RouteMeta;
};Then update 🤖 Prompt for AI Agents |
||
| ): this; | ||
|
|
||
|
|
@@ -164,15 +178,15 @@ export declare class H3 extends H3Core { | |
| /** | ||
| * Register a route handler for all HTTP methods. | ||
| */ | ||
| all(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
|
|
||
| get(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| post(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| put(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| delete(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| patch(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| head(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| options(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| connect(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| trace(route: string, handler: HTTPHandler, opts?: RouteOptions): this; | ||
| all(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
|
|
||
| get(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| post(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| put(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| delete(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| patch(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| head(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| options(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| connect(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| trace(route: string, handler: HTTPHandler<_ContextT>, opts?: RouteOptions): this; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extendContextfluent typing doesn't match runtime scoping.The declared signature in
src/types/h3.tsreturnsH3<_ContextT & Record<K, V>>(and the JSDoc on line 146 there says it returns "a new H3 instance"), implying that only handlers/middleware registered after the call see the extra key. The implementation, however, just callsthis.use(...)and returnsthis, so the appended middleware runs for every route on the instance — including ones registered beforeextendContext. Earlier routes get the key at runtime but theirevent.contextis typed without it, while the originalH3<_ContextT>reference still being held by callers becomes unsound.Two related concerns:
this(H3<_ContextT & Record<K, V>>cast aside) with a documented caveat.valOrFn: unknown+typeof valOrFn === "function"means a caller using the value-overload with a function value will have it invoked as a factory. Consider distinguishing by overload at the implementation level (e.g., separate methods, or anisFnflag) if that case matters.🤖 Prompt for AI Agents