Skip to content
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

Default type for Context and Handler prevents extension or override #583

Open
lostrouter opened this issue May 26, 2023 · 3 comments
Open

Comments

@lostrouter
Copy link

if i define a handler like so, and attempt to register it with other handlers, tsc will throw errors.

export const myHandler: Handler<
    unknown,
    { id: string }
> = async (
    c: OpenAPIBackendContext<unknown, { id: string }>,
    ctx: KoaContext,
): Promise<void> => {}
export const v1InternalAPI = new OpenAPIBackend({
    apiRoot: '/v1',
    definition: __dirname + '../../../api/service.v1.internal.yaml',
    // this is needed to honor the "format" field in the openapi definition
    customizeAjv: (originalAjv) => {
        addFormats(originalAjv);
        return originalAjv;
    },
});

// register your framework specific request handlers here
v1InternalAPI.register({
    postResponseHandler,
    validationFail,
    notFound,
    myHandler,
});

the errors

src/routes/v1/router.ts:27:5 - error TS2322: Type 'Handler<unknown, { id: string; }>' is not assignable to type 'Handler<any, UnknownParams, UnknownParams, UnknownParams, UnknownParams, Document>'.
  Property 'id' is missing in type 'UnknownParams' but required in type '{ id: string; }'.

27     myHandler,
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~

  src/controllers/myHandler.ts:12:7
    12     { id: string }
             ~~
    'id' is declared here.
  node_modules/openapi-backend/backend.d.ts:156:9
    156         [operationId: string]: Handler;
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    The expected type comes from this index signature.


Found 1 error in src/routes/v1/router.ts:27

what am i doing wrong here?

@kmvpvl
Copy link

kmvpvl commented May 26, 2023

Handler must have 3 parameters: context, Request,Response. You can send your own parameter after those ones.

f.e. async handler(ctx: Context, req: Request, res: Response, myObject: {id: string}, mySecondObject: ...)

@lostrouter
Copy link
Author

That would be true if i were using express. my project is using koa, which only has 2 arguments going into the handler.

@Powell-v2
Copy link

src/routes/v1/router.ts:27:5 - error TS2322: Type 'Handler<unknown, { id: string; }>' is not assignable to type 'Handler<any, UnknownParams, UnknownParams, UnknownParams, UnknownParams, Document>'.
Property 'id' is missing in type 'UnknownParams' but required in type '{ id: string; }'.

The error you are getting indicates that there's some kind of disjunction in typing since your custom { id: string} parameter does not contradict the Params generic expected by the Handler:

type UnknownParams = {
    [key: string]: string | string[];
}

export type Handler<
  RequestBody = any,
  Params = UnknownParams,
  Query = UnknownParams,
  Headers = UnknownParams,
  Cookies = UnknownParams,
  D extends Document = Document,
> = (context: Context<RequestBody, Params, Query, Headers, Cookies, D>, ...args: any[]) => any | Promise<any>;

Looking at the register method, the current signature suggests that Handler receives no generics at all:

public register(handlers: { [operationId: string]: Handler }): void;

Your custom Handler signature defines an object of a particular shape { id: string }, and while it does fit into the expected generic type { [key: string]: string | string[] }, the opposite is not the case.

It seems to me that we could overcome this issue by making the Handler in the register function aware of each handler's custom generics. The problems is that the registration is performed in bulk and I'm not 100% sure how to dynamically define generics for each handler in this case. On the other hand, doing so individually with the registerHandler method should be more or less straightforward:

registerHandler<
  RequestBody = any,
  Params = UnknownParams,
  Query = UnknownParams,
  Headers = UnknownParams,
  Cookies = UnknownParams,
  D extends Document = Document
>(
  operationId: string,
  handler: Handler<
    RequestBody = any,
    Params = UnknownParams,
    Query = UnknownParams,
    Headers = UnknownParams,
    Cookies = UnknownParams,
    D extends Document = Document
  >
): void;

Circling back to the example provided in the description:

type Body = unknown
type PathParams = { id: string }

const myHandler: Handler<Body, PathParams> = (
  context,
  ctx: KoaContext,
) => {}

api.registerHandler<Body, PathParams >('myHandler', myHandler)

Doing so connects the dots for TypeScript and will prevent type mismatch complaints.

@anttiviljami curios to hear your thoughts on the matter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants