Enhancement: [require-await] add option to ignore overrides methods

[kkimdev]

Before You File a Proposal Please Confirm You Have Done The Following...

• I have searched for related issues and found none that match my proposal.
• I have searched the current rule list and found no rules that match my proposal.
• I have read the FAQ and my problem is not listed.

My proposal is suitable for this project

• I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/require-await

Description

"require-await" rule asserts an existence of await inside async function. However, when a class method is inherited from a parent async method, sometimes it's inevitable to keep async signature even when the body doesn't have any await. Of course, one workaround is declaring it as a sync function then returning Promise.resolve(...) but that's less ergonomic.

So my proposal is adding an option called, let's say "AllowInOverriddenFunctions", that allows async functions without await for overridden functions. Please take a look at the example below.

Pass

abstract class A {
  abstract func(): Promise<undefined>;
}

class B extends A {
  override async func() {
    return undefined;
  }
}

Additional Info

No response

[Josh-Cena]

Duplicate (or at least highly related to) #7450

[bradzacher]

I'll be honest when I say that the other issue and this one should probably be closed cos it's not overly correct to use async as a side-effect to implicitly convert your return type to a promise.

There's no specific reason that an implementation of an interface or an override must also be async - it's just a "lazy" way to satisfy the contract of returning a promise.

I'm of two minds here as to whether or not this should be done. The entire point of the rule is to enforce the code explicitly does not leverage this implicit promise-wrapping behaviour and I don't really think that satisfying a contract should be valid for the rule?

Maybe behind an option but IDK. There's a lot of type work that would need to be done to resolve this information so it wouldn't be a cheap option.

[kkimdev]

@bradzacher What you mentioned - "There's no specific reason that an implementation of an interface or an override must also be async - it's just a "lazy" way to satisfy the contract of returning a promise." is absolutely true, though I'd say that in fact async itself was born mainly for ergonomic convenience anyways, i.e., no one has to use async-await, it does not enable any new capacity, it's just a lazy (or very easy) way to use Promise.

So if the ergonomic benefit of async is worth it, I think that having this option makes sense for the same reason.

[bradzacher]

it does not enable any new capacity

I would disagree because it's not just syntactic sugar for .then and .catch - it enables a vastly different style that can make promise-based code much, much easier to reason about.

So whilst, yes, you could write code without it - it vastly improves things.

OTOH adding async purely to satisfy the contract of a promise return is generally considered a code smell (which was the genesis for this rule!).

For example one could argue that the async is similarly necessary in this code:

type F = () => Promise<number>;
const fn: F = async () => 1;

Just like the overrides case, this case leverages async to lazily satisfy an external contact.

So if if the rule allows one - should it not support the other as well?

I don't think so. I think that the point of the rule is that you should avoid async purely as a lazy way to wrap with a promise and instead it's encouraging you to be explicit - I.e. It's encouraging code like async () => Promise.resolve(1) so the code you're writing is clear in the intent that you 100% meant to implement synchronous code in an asynchronous context.

The issue with ignoring this sort of case is that it's hard for a future reader to understand whether or not you really meant to use the keyword lazily or not.

[kkimdev]

Let me share our motivating example instead:

We're using rpc framework that generates ....Implementation stub typescript interface that contains rpc signatures from an rpc specification file, and the server is supposed to implement them. This is very similar to https://github.com/deeplay-io/nice-grpc/tree/master/packages/nice-grpc so I'll borrow nice-grpc as an example:

const exampleServiceImpl: ExampleServiceImplementation = {
  async exampleUnaryMethod1(request: ExampleRequest1): Promise<DeepPartial<ExampleResponse1>> {
    // implementation
  },
  async exampleUnaryMethod2(request: ExampleRequest2): Promise<DeepPartial<ExampleResponse2>> {
    // implementation
  },
  async exampleUnaryMethod3(request: ExampleRequest3): Promise<DeepPartial<ExampleResponse3>> {
    // implementation
  },
  // other rpc methods, so on
};

The rpc call signature is async because commonly the server has to make database or other network calls inside, and the interface should allow them, although not necessarily always used, depending on RPCs (e.g., returning from a memory cache or constants, simple synchronous computations, etc,..).

This is the server side codebase so it's very clear for our engineers that the top level RPCs are async and that's the way it is. There isn't much added clarity from converting non-awaiting rpc calls to sync-promise returning style in this context.

We have three options for these cases (non-awaiting rpc calls):

1. Continue to use async.
2. Convert them to sync-promose-returning style.
3. Change the generated server interface to accept both sync and async returns (e.g., () => Promise<Response> | () => Response)

We don't feel like 3 is worth the complexity, and we don't always have the ability to make upstream library changes. While 2 is doable but it's a bit more cumbersome than 1, without any benefit in this context, so we prefer 1.

There is also a different example but the theme is the same: The interface is async to allow async calls inside, but it's a matter of choice(of an implementation) rather than an expectation, and not using await is totally fine and doesn't have to be explicit.

I'm not claiming all the cases are like these, hence not suggesting as a default rule (though in our experience, we find that when someone makes a choice to make an async signature as a part of interface, that's usually a careful choice and rarely incorrect), but I think having an option is reasonable.

[JoshuaKGoldberg]

3. Change the generated server interface to accept both sync and async returns (e.g., () => Promise | () => Response)

We don't feel like 3 is worth the complexity

I think that's the right solution. The server-generated interfaces are forcing you to do something you don't need to do: explicitly return Promises from your functions. But there's nothing inherent to the code patterns that need your functions to be async / return Promises!

We've seen this also with async functions in frameworks such as Next.js. One library/framework says that a function must return Promise<T> rather than Promise<T> | T for simplicity, which forces users to unnecessarily provide async functions.

export const getServerSideProps: GetServerSideProps = () => {
  //         ~~~~~~~~~~~~~~~~~~
  // Type '() => { props: { movies: never[]; }; }' is not assignable to type 'GetServerSideProps<{}>'.
  //   Type '{ props: { movies: never[]; }; }' is missing the following properties
  //   from type 'Promise<GetServerSidePropsResult<{}>>':
  ...
};

As Josh said, this isn't quite a duplicate of #7450, but is highly related. Closing for the same reason.

tl;dr: the rule is right and the types are wrong. 😄