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

Is this library needed when using express-async-handler or express-async-router? #36

Closed
dko-slapdash opened this issue Oct 15, 2021 · 3 comments

Comments

@dko-slapdash
Copy link

https://www.npmjs.com/package/express-async-handler
https://www.npmjs.com/package/express-async-router

It sounds like using of one of the above libraries makes the usage of express-async-handler non-needed. Is it correct?

@kronicker
Copy link
Collaborator

kronicker commented Oct 16, 2021

In a way yes, this library like those you’ve mentioned aims to solve the same problem - not having to manually catch rejected promises in middlewares and send err to global handler.
However there are 2 main differences:

  1. This library also handles Router#param middleware and
  2. It patches express so you only need to import it once in your app. This is also beneficial since express v5 is going to support this out of the box so migrating from this library to express v5 is a matter of removing just one import/dependency.
    Whatever you go with it makes sense to use only one out of these 3.

@dko-slapdash
Copy link
Author

2 more cents/FYIs here:

  1. In the recent Express v4, express-async-errors can't be used together with typescript-eslint's no-misused-promises rule (it just doesn't allow passing async handlers to a callback which is expected to return void). Express authors made TS typing stricter, it doesn't accept async handlers anymore.
  2. Even when using and alternative, express-async-handler, there is a bug in no-misused-promises rule which doesn't always verify that the async handlers are treated properly, I reported it here: [no-misused-promises] checksVoidReturn=true doesn't catch the error when variadic arguments are used (like ...handlers: Array<() => void>) typescript-eslint/typescript-eslint#4015 - I found a work-around though, mentioned in the issue above.

Our story: after upgrading Express and TS, we had to stop using express-async-errors and switched to express-async-handler; to do so, we had to apply the work-around for no-misused-promises eslint rule mentioned above.

@dko-slapdash
Copy link
Author

dko-slapdash commented Oct 16, 2021

@kronicker JFYI, if you couple express-async-errors with TS typing which allows async handlers to be passed to express router matcher/handler (recent Express enforces all the handlers to return void and not a Promise), then it would make the library even better. (I understand that it currently has nothing to do with TS, but anyways.) Below is a code snippet which we used to work-around a bug in no-misused-promises eslint rule typescript-eslint/typescript-eslint#4015 - if adjusted accordingly, the similar approach may allow people to use async handlers, express-async-errors library, recent Express and no-misused-promises eslint rule together.

import { ParsedQs } from "qs";

// See https://github.com/typescript-eslint/typescript-eslint/issues/4015 to
// understand why. Once it's resolved (if it is), we can remove this file and
// this folder.

declare module "@types/express-serve-static-core" {
  export interface IRouterMatcher<
    T,
    Method extends
      | "all"
      | "get"
      | "post"
      | "put"
      | "delete"
      | "patch"
      | "options"
      | "head" = any
  > {
    <
      Route extends string,
      P = RouteParameters<Route>,
      ResBody = any,
      ReqBody = any,
      ReqQuery = ParsedQs,
      Locals extends Record<string, any> = Record<string, any>
    >(
      // tslint:disable-next-line no-unnecessary-generics (it's used as the default type parameter for P)
      path: Route,
      // tslint:disable-next-line no-unnecessary-generics (This generic is meant to be passed explicitly.)
      handler1: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler2?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler3?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler4?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler5?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>
    ): T;

    <
      Path extends string,
      P = RouteParameters<Path>,
      ResBody = any,
      ReqBody = any,
      ReqQuery = ParsedQs,
      Locals extends Record<string, any> = Record<string, any>
    >(
      // tslint:disable-next-line no-unnecessary-generics (it's used as the default type parameter for P)
      path: Path,
      // tslint:disable-next-line no-unnecessary-generics (This generic is meant to be passed explicitly.)
      handler1: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler2?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler3?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler4?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler5?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>
    ): T;

    <
      P = ParamsDictionary,
      ResBody = any,
      ReqBody = any,
      ReqQuery = ParsedQs,
      Locals extends Record<string, any> = Record<string, any>
    >(
      path: PathParams,
      // tslint:disable-next-line no-unnecessary-generics (This generic is meant to be passed explicitly.)
      handler1: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler2?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler3?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler4?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler5?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>
    ): T;

    <
      P = ParamsDictionary,
      ResBody = any,
      ReqBody = any,
      ReqQuery = ParsedQs,
      Locals extends Record<string, any> = Record<string, any>
    >(
      path: PathParams,
      // tslint:disable-next-line no-unnecessary-generics (This generic is meant to be passed explicitly.)
      handler1: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler2?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler3?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler4?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler5?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>
    ): T;
  }

  export interface IRouterHandler<T, Route extends string = string> {
    (
      handler1: RequestHandler<RouteParameters<Route>>,
      handler2?: RequestHandler<RouteParameters<Route>>,
      handler3?: RequestHandler<RouteParameters<Route>>,
      handler4?: RequestHandler<RouteParameters<Route>>,
      handler5?: RequestHandler<RouteParameters<Route>>
    ): T;

    (
      handler1: RequestHandlerParams<RouteParameters<Route>>,
      handler2?: RequestHandlerParams<RouteParameters<Route>>,
      handler3?: RequestHandlerParams<RouteParameters<Route>>,
      handler4?: RequestHandlerParams<RouteParameters<Route>>,
      handler5?: RequestHandlerParams<RouteParameters<Route>>
    ): T;

    <
      P = RouteParameters<Route>,
      ResBody = any,
      ReqBody = any,
      ReqQuery = ParsedQs,
      Locals extends Record<string, any> = Record<string, any>
    >(
      // tslint:disable-next-line no-unnecessary-generics (This generic is meant to be passed explicitly.)
      handler1: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler2?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler3?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler4?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler5?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>
    ): T;

    <
      P = RouteParameters<Route>,
      ResBody = any,
      ReqBody = any,
      ReqQuery = ParsedQs,
      Locals extends Record<string, any> = Record<string, any>
    >(
      // tslint:disable-next-line no-unnecessary-generics (This generic is meant to be passed explicitly.)
      handler1: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler2?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler3?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler4?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler5?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>
    ): T;

    <
      P = ParamsDictionary,
      ResBody = any,
      ReqBody = any,
      ReqQuery = ParsedQs,
      Locals extends Record<string, any> = Record<string, any>
    >(
      // tslint:disable-next-line no-unnecessary-generics (This generic is meant to be passed explicitly.)
      handler1: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler2?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler3?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler4?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler5?: RequestHandler<P, ResBody, ReqBody, ReqQuery, Locals>
    ): T;

    <
      P = ParamsDictionary,
      ResBody = any,
      ReqBody = any,
      ReqQuery = ParsedQs,
      Locals extends Record<string, any> = Record<string, any>
    >(
      // tslint:disable-next-line no-unnecessary-generics (This generic is meant to be passed explicitly.)
      handler1: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler2?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler3?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler4?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>,
      handler5?: RequestHandlerParams<P, ResBody, ReqBody, ReqQuery, Locals>
    ): T;
  }
}

image

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

2 participants