Type definitions are wrong, don't allow Promise to be returned (caught by @typescript-eslint)

[rdsedmundo]

Env:

TypeScript: 4.1.3
Express: 4.17.1
Node: 14.16.0
@typescript-eslint/eslint-plugin: 4.17.0
@typescript-eslint/parser: 4.17.0

Repro:

import express from 'express';
import createRouter from 'express-promise-router';

export const router = createRouter();

router.get(
  '/',
  async (req: express.Request, res: express.Response): Promise<string> => {
    res.sendStatus(200);
  },
);

export const server = express().use(router);

Produces:

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

This is because the type definitions from express-promise-router are just using the express.Router interface which doesn't support Promises (the very reason why this package exists).

[mormahr]

Hi,

we don’t send out values returned by the promises. We want to avoid values being implicitly returned by a promise chain to end up at the client. It would also be hard to support status codes and the like. We run the promises and forward errors, but you have to send the values yourself. See the section „Why aren't promise values sent to the client“ in the readme.

If you want to send a string, like I assume you wanted in your example, you call res.send().

import express from 'express';
import createRouter from 'express-promise-router';

export const router = createRouter();

router.get(
  '/',
  async (req: express.Request, res: express.Response): Promise<void> => {
    res.status(200).send('Hello World');
  },
);

export const server = express().use(router);

I hope this cleared up any confusion.
Don’t hesitate to ask if you have any other issues.

As an aside:
I didn‘t originally contribute the type definitions, so I haven’t checked if they support the two return values we actually do allow ('next' and 'route'), but I’ll check that next week.

[rdsedmundo]

Thanks for the reply, @mormahr.

I'm aware of the Promise/response behavior, but this is not the issue. As soon as you mark a function with the async keyword it automatically returns a Promise.

[mormahr]

Yes and that's correct, but it has to return Promise<void> like in my example.

[mormahr]

I think i understand what you mean now. It's not a TypeScript error, it's that ts eslint thinks the promise is unused. I'll have a look later.

[rdsedmundo]

Yep, exactly. It states that because the return signature from the Router interface doesn't say that a Promise is returned.

[mormahr]

I added a failing test case in #231.
I feel like the only way to fix this is to duplicate part of the express typings to overwrite all the handler types.

[mobeigi]

Would be nice to merge this so consumers don't have to disable the lint rule for every async router. Its a useful typescript eslint rule so you don't want to disable it globally.

[mormahr]

There is nothing to merge. I wrote a failing test case but actually solving this isn't trivial as (as far as I can tell) we need to duplicate lots of definitions from @types/express. I haven't had the time to look into this again since then.

[mormahr]

From what I've tried, the problem is essentially that in the type definition of express-serve-static-core the return type of a handler is defined as void where we would need it to be void | Promise<void>. But since this type is buried so far down the chain of types, it's hard to replace it.

I might contact the maintainers of the @types/express and @types/express-serve-static-core and ask for advice/help.