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

Lint error with TypeScript #35

Open
royalpinto opened this issue Jun 1, 2021 · 8 comments
Open

Lint error with TypeScript #35

royalpinto opened this issue Jun 1, 2021 · 8 comments

Comments

@royalpinto
Copy link

Using the async handler with this lib leads to the following lint error as the return type of async handler is a Promise<> and the expected one is just a void.

Error Message:

Promise returned in function argument where a void return was expected. (@typescript-eslint/no-misused-promises)

I am not sure if the type definition of the express handler can be changed in this lib's definitions. Create a ticket just in case if it's possible.

@Bikash9609
Copy link

Bikash9609 commented Sep 13, 2021

Me too getting the same error. So I tried doing something like this:
export const tempName = (req: Request, res: Response, next: NextFunction) => async () => { // everything else with await }

And then the linting error is gone.

@AxelGreen
Copy link

Any updates here?

@Bikash9609
Copy link

Bikash9609 commented Feb 23, 2023

Any updates here?

Can you share some code where you are getting this lint error? I think I know the solution

@AxelGreen
Copy link

Error message

ESLint: Promise returned in function argument where a void return was expected.(@typescript-eslint/no-misused-promises)

Code:

import express, { Request, Response } from 'express';
import 'express-async-errors';

async function asyncThrowError(): Promise<void> {
    await Promise.all([]);
    throw new Error('Test error');
}

async function main(): Promise<void> {
    // TODO: remove later
    await Promise.all([]);

    const app = express();

    app.get('/async-error', async (_request: Request, response: Response) => {
        await asyncThrowError();
        response.status(200).json({ success: true });
    });

    app.listen(40512);
}

main()
    .then(() => {
        // eslint-disable-next-line no-console
        console.log('Application started');
    })
    .catch((error: Error) => {
        // eslint-disable-next-line no-console
        console.error('Application startup error', { error });
    });

CleanShot 2023-02-23 at 16 00 01@2x

@Bikash9609
Copy link

Bikash9609 commented Feb 24, 2023

app.get('/async-error', async (_request: Request, response: Response): Promise<void> // Added this => {
    await asyncThrowError();
    response.status(200).json({ success: true });
});

or

app.get('/async-error', async (_request: Request, response: Response) => {
    await asyncThrowError();
    return response.status(200).json({ success: true }); // Added return
});

Let me know if one of them works.

@AxelGreen
Copy link

Thank you for your suggestions!

Unfortunately, both versions give me the same issue as before.

CleanShot 2023-02-24 at 11 48 36@2x

@code0monkey1
Copy link

Same error . Plus , it does not tend to handle the async errors resulting from using Multer for file uploads

@Alexsey
Copy link

Alexsey commented May 24, 2024

I have the solution!

EDIT: Unfortunately, fixing the RequestHandler type is only part of the solution:

  • we cannot fix the ErrorRequestHandler type because it's declared as type, not as interface. I assume, we can discuss this change with the maintainer of the types package
  • the express application Application type and the router IRouter type are also affected by the fix, as they are extended from the RequestHandler type. So they will trigger the same eslint error when using them e.g. as an argument for https.createServer

At the moment, this is above the time budget I have for the task. Maybe someone else would like to pick it up from here

The partial solution

  • It will not remove the eslint error from error-handling routes
  • It may introduce a few new instances of this eslint error. Yet, probably much less than will eliminate

Create or extend an existing .d.ts file in your project with the following code (e.g. types-override.d.ts):

import type { Request, Response } from 'express';
import type { ParamsDictionary, Query } from 'express-serve-static-core';
import type { NextFunction } from 'express-serve-static-core';

declare module 'express-serve-static-core' {
  export interface RequestHandler<
    P = ParamsDictionary,
    ResBody = any,
    ReqBody = any,
    ReqQuery = Query,
    LocalsObj extends Record<string, any> = Record<string, any>
  > {
    (
      req: Request<P, ResBody, ReqBody, ReqQuery, LocalsObj>,
      res: Response<ResBody, LocalsObj>,
      next: NextFunction
    ): void | Promise<void>;
  }
}
  • In case you are creating a new file and your tsconfig.json has the "include" section then ensure that the file is covered by the patterns of the section
  • For the TypeScript versions older than 3.8 you can replace import type with just import

You are done!

The type is fixed, so you will see no @typescript-eslint/no-misused-promises) error for your routes

Comments on the implementation

The conflict we are having here is with the type RequestHandler of express, which is extended from the type RequestHandler of the express-serve-static-core - its return type is void, and we are providing a function with the return type Promise<void>. If we patch the RequestHandler of the express it would become incompatible with the root type, this is why we are extending directly the root type

The feature we are using here is called merging interfaces it allows extending existing interfaces by re-declaring them

So, here we duplicate the definition of the RequestHandler type from the express-serve-static-core but change the return type from void to void | Promise<void>

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

5 participants