Remove no-return-await rule

[Bamieh]

What rule do you want to change?

no-return-await.

The rule description body of the rule not longer valid. The rule states that this syntax is useless although it is a very valuable implementation:

It has better performance than hand-written promises, and more importantly it produces better debugging stack traces.

Read full details about the benefits of using return await in this answer: https://stackoverflow.com/questions/44806135/why-no-return-await-vs-const-x-await/44806230#44806230

How will the change be implemented? (New option, new default behavior, etc.)?

I'd suggest removing the rule and introducing a new one that throws if a promise is not awaited inside an async function.

A saner approach is to update the rule description to reflect that this longer is no longer correct and deprecate it in code.

Please provide some example code that this change will affect:

return await <Promise>

What does the rule currently do for this code?

The rule states that this rule is useless although it is a very valuable implementation, it increases performance, produces better debugging stack traces, and it explicitly denotes that the returned function is a promise.

What will the rule do after it's changed?

I'd vote for removing the rule and creating another one with the exact opposite behavior. IDK how you deal with backwards compatibility.

Are you willing to submit a pull request to implement this change?

No

[coderaiser]

There is an article about this in v8 blog: https://v8.dev/blog/fast-async.
It states that return await is faster and produces better call stacks in v8 engine.

Code transformer with built-int eslint support putout has a plugin add-return-await, it can help to find and add await to return statements, when it's argument is promise.

[ljharb]

It’s still conceptually redundant, and shouldn’t be there. The rule is a good one, and a temporary speed difference in one engine is not a good reason to alter coding patterns.

[ljharb]

Also, nothing in that article talks about return await specifically; it’s just that the performance hit May no longer apply - it’s still bad code, because it’s conceptually redundant.

[Bamieh]

@ljharb it offers better debugging. You can consider the performance improvement as a nice side effect, however this is not what the rule states.

Please read this section in the article: https://v8.dev/blog/fast-async#improved-developer-experience

or follow this example:

    async function foo() {
      return bar();
    }
    
    async function bar() {
      await Promise.resolve();
      throw new Error('BEEP BEEP');
    }
    
    foo().catch(error => console.log(error.stack));


    Error: BEEP BEEP
        at bar (<anonymous>:7:9)

Note that by calling return bar(); foo() function call does not appear at all in the error stack. Changing it to return await bar(); gives a much better error stack output:

    async function foo() {
      return await bar();
    }

    Error: BEEP BEEP
        at bar (<anonymous>:7:9)
        at async foo (<anonymous>:2:10)

The description of this rule is not valid anymore and at the very least should be changed to reflect the current state of things.

Inside an async function, return await is seldom useful. Since the return value of an async function is always wrapped in Promise.resolve, return await doesn’t actually do anything except add extra time before the overarching Promise resolves or rejects.

This is wrong. It is useful. It provides better debugging experience, and it is a code style some do prefer having as it explicitly shows that this function call returns a Promise.

It helps in refactoring code in the future and understanding where async code is happning. But again we are now discussing code style which I dont think we should be arguing about; Some prefer using await before returning a promise to explicitly show that this function call returns a promise, and some dont use it as they see it is not needed (or as you phrased it; redundant) and they can use this rule, but I think the description should be changed to reflect that they DO get benefits using the return await syntax.

This rule aims to prevent a likely common performance hazard due to a lack of understanding of the semantics of async function.

This is not a "performance hazard" anymore, on the contrary.

[ljharb]

I would prefer my code to be cleaner by always avoiding return await, and no performance or debugging argument outweighs that for me. Disable the rule if you want, but that’s not an argument to remove it.

[kaicataldo]

PRs to update documentation are always welcome! I think it could be useful to include some of this information on the rules page.

[ilyavolodin]

My position is that debugging experience improvement is implementation detail of the engine. I see no reason why it should be required for correctly recording async function into stack trace. In my opinion, that implementation is flawed. There shouldn't be a reason to wrap a promise inside another promise to provide correct stack traces. Setting aside performance and stack traces, I think double wrapping return value in promises doesn't make sense to me. I would be against removing this rule. At the very most, I could be convinced to rename this rule to return-await and provide an option to either require it, or prevent it.

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

[dbjorge]

I think the analysis of this rule in standard/standard#1442 brings up a second, more important reason that would make me prefer to see this rule either deprecated or at least not-recommended-by-default, which is that it sets a trap for an assortment of different types of refactoring (see that issue for more details). Frankly, I think the refactoring traps are enough that I think it would make more sense for the default recommendation to encourage the await.

[dbjorge]

Further, I agree with the argument that the difference in stack trace usability and performance should be treated as an implementation detail, but I think that this should encourage picking the choice that leads to code which is easiest for a maintainer to understand regardless of whether some implementations might use an extra wrapping layer with the extra await, and I think making the sync/async nature more obvious to a reader has a bigger impact on improving readability than removing the redundant token does.

async function doThings() {
  const a = await thingA();
  const b = thingB(a);
  // This *looks* synchronous, but actually the reader can't tell without looking up function signatures.
  // The real behavior would be more obvious to a future maintainer with the await.
  return thingC(b);
}

[mightyiam]

Should this be re-opened, please?

[mightyiam]

Not for the purpose of removing it, because some are interested in it, but perhaps for a different solution, like documentation changes?

[LinusU]

It’s still conceptually redundant, and shouldn’t be there.

I really don't think it is though. return await foo and return foo is really two very different things.

My position is that debugging experience improvement is implementation detail of the engine. I see no reason why it should be required for correctly recording async function into stack trace. In my opinion, that implementation is flawed.

It isn't an implementation detail though, if the engine did "correctly recording async function into stack trace" when you returned a thenable from your async function, that would actually be wrong since that function shouldn't be on the call stack anymore.

There shouldn't be a reason to wrap a promise inside another promise to provide correct stack traces.

I feel like you are contradicting yourself again here since doing return foo instead of return await foo would actually be what's wrapping it in another Promise. If return ... in an async function desugars to return Promise.resolve(...) then writing return foo where foo is already a promise would be wrapping it in double promises.

Would be happy if we could open this up again for further discussion ☺️

[kaicataldo]

Feel free to make a new issue if you’d like to advocate for this change.