From b558579dbac540e454f88c848902b19b5223a6b9 Mon Sep 17 00:00:00 2001 From: Danilo Alonso Date: Tue, 21 Jul 2026 14:30:04 -0400 Subject: [PATCH] fix(types): make request Refs covariant Typed lifecycle methods failed to attach to any surface whose Refs were not identical. Method is now bivariant, Request/ResponseToolkit covariant with an empty-bag default; paramsArray was mistyped as keys. --- lib/types/request.d.ts | 6 +- lib/types/response.d.ts | 2 +- lib/types/route.d.ts | 20 +++--- lib/types/server/auth.d.ts | 8 +-- lib/types/server/ext.d.ts | 2 +- lib/types/server/server.d.ts | 4 +- lib/types/utils.d.ts | 16 +++-- test/types/index.ts | 128 +++++++++++++++++++++++++++++++++-- 8 files changed, 153 insertions(+), 33 deletions(-) diff --git a/lib/types/request.d.ts b/lib/types/request.d.ts index 23452c54e..82b174414 100644 --- a/lib/types/request.d.ts +++ b/lib/types/request.d.ts @@ -199,7 +199,7 @@ export interface RequestInfo { * * fingerprint - the route internal normalized string representing the normalized path. * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-requestroute) */ -export interface RequestRoute { +export interface RequestRoute { /** the route HTTP method. */ method: Exclude, 'head'> | '*'; @@ -314,7 +314,7 @@ export type MergeRefs = MergeType; * HTTP server callback (which is available via [request.raw.req](https://github.com/hapijs/hapi/blob/master/API.md#request.raw)). The request properties change throughout * the request [lifecycle](https://github.com/hapijs/hapi/blob/master/API.md#request-lifecycle). */ -export interface Request extends Podium { +export interface Request extends Podium { /** * Application-specific state. Provides a safe place to store application data without potential conflicts with the framework. Should not be used by plugins which should use plugins[name]. * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-requestapp) @@ -404,7 +404,7 @@ export interface Request extends Podium { /** * An array containing all the path params values in the order they appeared in the path. */ - readonly paramsArray: keyof MergeRefs['Params'] | string[]; + readonly paramsArray: string[]; /** * The request URI's pathname component. diff --git a/lib/types/response.d.ts b/lib/types/response.d.ts index f7c131787..ec6c5e394 100644 --- a/lib/types/response.d.ts +++ b/lib/types/response.d.ts @@ -412,7 +412,7 @@ export interface Auth< * document the h notation is used. It is named in the spirit of the RethinkDB r method, with h for hapi. * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#response-toolkit) */ -export interface ResponseToolkit { +export interface ResponseToolkit { /** * A response symbol. When returned by a lifecycle method, the request lifecycle skips to the finalizing step * without further interaction with the node response stream. It is the developer's responsibility to write diff --git a/lib/types/route.d.ts b/lib/types/route.d.ts index f85cf00d8..92e4f33c6 100644 --- a/lib/types/route.d.ts +++ b/lib/types/route.d.ts @@ -336,12 +336,12 @@ export interface RouteOptionsPayload { /** * For context [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-routeoptionspre) */ -export type RouteOptionsPreArray = RouteOptionsPreAllOptions[]; +export type RouteOptionsPreArray = RouteOptionsPreAllOptions[]; /** * For context [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-routeoptionspre) */ -export type RouteOptionsPreAllOptions = RouteOptionsPreObject | RouteOptionsPreObject[] | Lifecycle.Method; +export type RouteOptionsPreAllOptions = RouteOptionsPreObject | RouteOptionsPreObject[] | Lifecycle.Method; /** * An object with: @@ -350,7 +350,7 @@ export type RouteOptionsPreAllOptions = Ro * * failAction - A failAction value which determine what to do when a pre-handler method throws an error. If assign is specified and the failAction setting is not 'error', the error will be assigned. * For context [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-routeoptionspre) */ -export interface RouteOptionsPreObject { +export interface RouteOptionsPreObject { /** * a lifecycle method. */ @@ -358,7 +358,7 @@ export interface RouteOptionsPreObject { /** * key name used to assign the response of the method to in request.pre and request.preResponses. */ - assign?: keyof MergeRefs['Pres'] | undefined; + assign?: Extract['Pres'], string> | (string & {}) | undefined; /** * A failAction value which determine what to do when a pre-handler method throws an error. If assign is specified and the failAction setting is not 'error', the error will be assigned. */ @@ -640,7 +640,7 @@ export interface RouteOptionsValidate { state?: RouteOptionsResponseSchema | undefined; } -export interface CommonRouteProperties { +export interface CommonRouteProperties { /** * Application-specific route configuration state. Should not be used by plugins which should use options.plugins[name] instead. * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-routeoptionsapp) @@ -886,7 +886,7 @@ export interface AuthSettings { access?: AccessSetting[] | undefined; } -export interface RouteSettings extends CommonRouteProperties { +export interface RouteSettings extends CommonRouteProperties { auth?: AuthSettings | undefined; } @@ -894,7 +894,7 @@ export interface RouteSettings extends Com * Each route can be customized to change the default behavior of the request lifecycle. * For context [See docs](https://github.com/hapijs/hapi/blob/master/API.md#route-options) */ -export interface RouteOptions extends CommonRouteProperties { +export interface RouteOptions extends CommonRouteProperties { /** * Route authentication configuration. Value can be: * false to disable authentication if a default strategy is set. @@ -915,14 +915,14 @@ export interface RulesInfo { vhost: string; } -export interface RulesOptions { +export interface RulesOptions { validate: { schema?: ObjectSchema['Rules']> | Record['Rules'], Schema> | undefined; options?: ValidationOptions | undefined; }; } -export interface RulesProcessor { +export interface RulesProcessor { (rules: MergeRefs['Rules'] | null, info: RulesInfo): Partial> | null; } @@ -943,7 +943,7 @@ type RouteDefMethods = Exclude, 'HEAD' | * * rules - route custom rules object. The object is passed to each rules processor registered with server.rules(). Cannot be used if route.options.rules is defined. * For context [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-serverrouteroute) */ -export interface ServerRoute { +export interface ServerRoute { /** * (required) the absolute path used to match incoming requests (must begin with '/'). Incoming requests are compared to the configured paths based on the server's router configuration. The path * can include named parameters enclosed in {} which will be matched against literal values in the request as described in Path parameters. For context [See diff --git a/lib/types/server/auth.d.ts b/lib/types/server/auth.d.ts index afc6ba92e..e567b5ba4 100644 --- a/lib/types/server/auth.d.ts +++ b/lib/types/server/auth.d.ts @@ -26,7 +26,7 @@ export type ServerAuthScheme< // tslint:disable-next-line no-unnecessary-generics Options extends ServerAuthSchemeOptions = ServerAuthSchemeOptions, // tslint:disable-next-line no-unnecessary-generics - Refs extends ReqRef = ReqRefDefaults + Refs extends ReqRef = {} > = (server: Server, options?: Options) => ServerAuthSchemeObject; export interface ServerAuthSchemeObjectApi {} @@ -36,7 +36,7 @@ export interface ServerAuthSchemeObjectApi {} * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#authentication-scheme) */ -export interface ServerAuthSchemeObject { +export interface ServerAuthSchemeObject { /** * optional object which is exposed via the [server.auth.api](https://github.com/hapijs/hapi/blob/master/API.md#server.auth.api) object. */ @@ -155,7 +155,7 @@ export interface ServerAuth { */ scheme < - Refs extends ReqRef = ReqRefDefaults, + Refs extends ReqRef = {}, Options extends object = {} // tslint:disable-next-line no-unnecessary-generics >(name: string, scheme: ServerAuthScheme): void; @@ -197,5 +197,5 @@ export interface ServerAuth { * entity, or other route properties. */ // tslint:disable-next-line no-unnecessary-generics - verify (request: Request): Promise; + verify (request: Request): Promise; } diff --git a/lib/types/server/ext.d.ts b/lib/types/server/ext.d.ts index 3f4133fd0..b3dd9a5e3 100644 --- a/lib/types/server/ext.d.ts +++ b/lib/types/server/ext.d.ts @@ -65,7 +65,7 @@ export interface ServerExtEventsObject { options?: ServerExtOptions | undefined; } -export interface RouteExtObject { +export interface RouteExtObject { method: Lifecycle.Method; options?: ServerExtOptions | undefined; } diff --git a/lib/types/server/server.d.ts b/lib/types/server/server.d.ts index 4d64710e1..4e607a5c2 100644 --- a/lib/types/server/server.d.ts +++ b/lib/types/server/server.d.ts @@ -621,7 +621,7 @@ export class Server { * Note that the options object is deeply cloned (with the exception of bind which is shallowly copied) and cannot contain any values that are unsafe to perform deep copy on. * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-serverrouteroute) */ - route (route: ServerRoute | ServerRoute[]): void; + route (route: ServerRoute | ServerRoute[]): void; /** * Defines a route rules processor for converting route rules object into route configuration where: @@ -642,7 +642,7 @@ export class Server { * @return void * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-serverrulesprocessor-options) */ - rules ( + rules ( processor: RulesProcessor, options?: RulesOptions | undefined ): void; diff --git a/lib/types/utils.d.ts b/lib/types/utils.d.ts index feb342e0a..3f855b0f3 100644 --- a/lib/types/utils.d.ts +++ b/lib/types/utils.d.ts @@ -58,14 +58,20 @@ export namespace Lifecycle { * * err - an error object available only when the method is used as a failAction value. */ type Method< - Refs extends ReqRef = ReqRefDefaults, + Refs extends ReqRef = {}, R extends ReturnValue = ReturnValue - > = ( + > = { + // Declared as a method and immediately indexed so signature comparisons are + // bivariant (method signatures are exempt from strictFunctionTypes). A method + // typed with narrower Refs is an assertion about what validation guarantees, + // so it must remain assignable to surfaces typed with default or wider Refs. + bivariance( this: MergeRefs['Bind'], request: Request, h: ResponseToolkit, err?: Error | undefined - ) => R; + ): R; + }['bivariance']; /** * Each lifecycle method must return a value or a promise that resolves into a value. If a lifecycle method returns @@ -81,8 +87,8 @@ export namespace Lifecycle { * - a promise object that resolve to any of the above values * For more info please [See docs](https://github.com/hapijs/hapi/blob/master/API.md#lifecycle-methods) */ - type ReturnValue = ReturnValueTypes | (Promise>); - type ReturnValueTypes = + type ReturnValue = ReturnValueTypes | (Promise>); + type ReturnValueTypes = (null | string | number | boolean) | (Buffer) | (Error | Boom) | diff --git a/test/types/index.ts b/test/types/index.ts index ec1212fcc..b5cb75b08 100644 --- a/test/types/index.ts +++ b/test/types/index.ts @@ -478,15 +478,12 @@ interface MyRouteRefs { Query: { expand: string }; } -// KNOWN LIMITATION: Request is not assignable to Request -// because TypeScript checks generic interface compatibility invariantly when -// the generic appears in contravariant positions (e.g. lifecycle method parameters). -// Workaround: use a generic function like processAuthGeneric above instead -// of concrete Request (no generic) for helper functions that need to accept -// requests with different Refs. +// FIXED: Request is covariant in Refs (`out` annotation, default of `{}`), +// so a request with narrower refs is assignable to helpers typed with plain +// Request. The generic-function workaround (processAuthGeneric above) is no +// longer required. export function issueConcreteVsGeneric(req: Request): void { - // @ts-expect-error - Known TS limitation: Request not assignable to Request concreteHelper(req); } @@ -515,3 +512,120 @@ const issueStateAny: ServerRoute = { return 'ok'; } }; + +// ----------------------------------------------------------------------------- +// ISSUE 7: Refs mismatches between lifecycle methods and their surfaces +// +// FIXED: Lifecycle.Method is bivariant (method-signature exemption from +// strictFunctionTypes) and Request/ResponseToolkit are covariant in Refs with +// a default of `{}`. Consequences, each exercised below: +// - a method typed with narrower Refs attaches to surfaces typed with the +// defaults: server.ext(), route-level ext, failAction, pre; +// - a route typed with a superset of refs accepts methods typed with any +// subset of those refs (reusable pres); +// - an untyped route infers its Refs from a typed handler; +// - contradictory refs and default-to-narrow flows still fail. +// ----------------------------------------------------------------------------- + +interface VariancePayload { + email: string; +} + +interface VarianceUser { + id: string; +} + +const typedPayloadMethod: Lifecycle.Method<{ Payload: VariancePayload }> = (request) => { + + check.type(request.payload); + return 'ok'; +}; + +const typedPreMethod: Lifecycle.Method<{ Pres: { user: VarianceUser } }> = (request) => { + + check.type(request.pre.user); + return 'ok'; +}; + +// A method with narrower Refs attaches to default-typed extension points + +server.ext('onPreHandler', typedPayloadMethod); + +const varianceDefaultSurfaces: ServerRoute = { + method: 'POST', + path: '/variance/default-surfaces', + options: { + validate: { + payload: true, + failAction: typedPayloadMethod + }, + ext: { + onPreHandler: { method: typedPayloadMethod } + }, + handler: () => 'ok' + } +}; + +// A route typed with the superset of refs accepts subset-typed methods + +const varianceSuperset: ServerRoute<{ + Payload: VariancePayload; + Pres: { user: VarianceUser }; +}> = { + method: 'POST', + path: '/variance/superset', + options: { + pre: [ + { method: typedPayloadMethod, assign: 'user' }, + { method: typedPreMethod } + ], + handler: typedPayloadMethod + } +}; + +// An untyped route infers its Refs from the typed methods it is given + +server.route({ + method: 'POST', + path: '/variance/inferred', + options: { + pre: [{ method: typedPayloadMethod, assign: 'login' }], + handler: typedPayloadMethod + } +}); + +// paramsArray holds the param VALUES in path order — always strings + +const varianceParamsArray: ServerRoute = { + method: 'GET', + path: '/variance/{id}', + handler: (request, h) => { + + check.type(request.paramsArray); + return 'ok'; + } +}; + +// Contradictory refs on the same slot must still fail + +const conflictingMethod: Lifecycle.Method<{ Payload: { count: number[] } }> = () => 'ok'; + +const varianceConflict: ServerRoute<{ Payload: VariancePayload }> = { + method: 'POST', + path: '/variance/conflict', + options: { + pre: [ + // @ts-expect-error - contradictory Payload refs are not assignable + { method: conflictingMethod, assign: 'user' } + ], + handler: typedPayloadMethod + } +}; + +// A default request must still not satisfy narrower refs + +export function varianceDefaultToNarrow(req: Request): void { + + // @ts-expect-error - Request (defaults) does not satisfy narrower refs + check.type>(req); +}