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

[quirrel/next] typing error in Queue object #928

Closed
thecodedrift opened this issue Jan 10, 2022 · 1 comment · Fixed by #931
Closed

[quirrel/next] typing error in Queue object #928

thecodedrift opened this issue Jan 10, 2022 · 1 comment · Fixed by #931

Comments

@thecodedrift
Copy link
Contributor

thecodedrift commented Jan 10, 2022

Bug Report

Current Behavior

Argument of type 'Queue<Properties>' is not assignable to parameter of type 'NextApiHandler<any>'.
  Type 'Queue<Properties>' provides no match for the signature '(req: NextApiRequest, res: NextApiResponse<any>): void | Promise<void>'.ts(2345)

Input Code
Discovered when doing a Sentry integration

export default withSentry(Queue<Properties>("api/queues/<redacted>", async (job) => {
  // ...
}));

Expected behavior/code
I didn't expect a typescript error, as quirrel/next should be returning an interface that extends NextApiHandler. Since we don't want to make a hard dependency on next, we can just provide sufficient type overlap.

Possible Solution

/*
Conversion of type 'Queue<Properties>' to type 'NextApiHandler<any>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Queue<Properties>' provides no match for the signature '(req: NextApiRequest, res: NextApiResponse<any>): void | Promise<void>'
*/

Since Queue<Payload> is unique per integration, my suggestion would be to use a union type to add the Quirrel properties. (I'm pretty mediocre at complex types, so this is my best approximation)

@https://github.com/quirrel-dev/quirrel/blob/main/src/next.ts#L35

type QueueClient<Payload> = Omit<
  QuirrelClient<Payload>,
  "respondTo" | "makeRequest"
  >;

type QuirrelNextHandler = (req: any, res: any) => void | Promise<void>;

export type Queue<Payload> = QuirrelNextHandler & QueueClient<Payload>;

I was able to verify this typing by putting the following code in locally:

type QuirrelNextHandler = (req: any, res: any) => void | Promise<void>;
type QueueFixed<Payload=any> = QuirrelNextHandler & Queue<Payload>;

export const MyQueueName: QueueFixed = Queue<Properties>("api/queues/<redacted>", async (job) => {
  // ...
}); // <= must enqueue off this object
export default withSentry(MyQueueName); // <= route handling

edit Unfortunately, any other next-style wrappers will also lose access to the queue object. I've updated the workaround code to reflect what I needed to do to get sentry integration working

@thecodedrift
Copy link
Contributor Author

For workarounds, here is the additional.d.ts currently being used

import {
  DefaultJobOptions,
  Queue as WrongTypedQueue,
  QuirrelJobHandler,
} from "quirrel/next";

// https://github.com/quirrel-dev/quirrel/issues/928
declare module "quirrel/next" {
  type QuirrelNextHandler = (req: any, res: any) => void | Promise<void>;
  type PatchedQueue<Payload = any> = QuirrelNextHandler & WrongTypedQueue<Payload>;
  export function Queue<Payload>(
    route: string,
    handler: QuirrelJobHandler<Payload>,
    defaultJobOptions?: DefaultJobOptions
  ): PatchedQueue<Payload>;
}

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

Successfully merging a pull request may close this issue.

1 participant