Configs: include return-await in recommended-type-checked preset

[aseemk]

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 read the FAQ and my problem is not listed.

Description

I recently debugged a difficult bug in my code where I was try-catching an async call that I was returning right away.

async function asyncFoo() {
  try {
    return asyncBar() // no await
  } catch (err) {
    // so my error handling code here wasn't getting hit
  }
}

I was surprised when I finally figured this out, because I love and rely on e.g. no-floating-promises to catch missing awaits, but it hadn't caught this one.

I found that return-await exists to help here, but was then also surprised that it wasn't included in the recommended-type-checked preset that we use.

I thought the whole purpose of that preset was to at least help ensure correctness (separate from style) — and this was definitely a correctness issue.

I've now enabled return-await in my codebase, and am feeling great. But can I suggest adding this to recommended-type-checked to prevent other developers from hitting easily preventable bugs like this?

Thank you for your consideration and for a great tool!

Impacted Configurations

recommended-type-checked

Additional Info

No response

[JoshuaKGoldberg]

Heh, I personally prefer this, but have memories of people getting ✨ very upset ✨ over it. Maybe we should put it in strict to start?

[Josh-Cena]

What did people get upset about with return-await? Returning an unawaited promise inside a try catch seems to invite for troubles. I also don't know why people ever want to return unawaited promises, except for very simple wrappers—fun fact, awaiting the promise does not make anything slower; it may actually make code faster than if you return the promise directly!

[JoshuaKGoldberg]

I don't have many primary resources that I can find in 5 minutes of searching 😞 but vaguely:

• The venerable https://jakearchibald.com/2017/await-vs-return-vs-return-await explicitly says "Outside of try/catch blocks, return await is redundant - and that article still has good SEO
• When https://eslint.org/docs/latest/rules/no-return-await was written, IIRC it was taken to heart by many
• Scattered old tweets from searching for return await javascript show folks occasionally suggesting a preference for less code (return a over return await a) and/or not wanting to pollute the call stack
• Edit: this one is fun too, Docs: no-return-await deprecation notice leads to a wrong conclusion eslint/eslint#18166

it may actually make code faster than if you return the promise directly!

Do you have references?

Maybe relevant: eslint/eslint#18166

[kirkwaiblinger]

I'm big +1 to this, but, I am also concerned that the current rule is too opinionated, and would cause a lot of unnecessary flagging in existing codebases. I'd propose we have an unopinionated option that only flags on the parts of the rule that impact correctness, if we are to add the rule to recommended; see #9030. Note that all the disagreements center around the portion of the rule that I'm proposing to disable.

[bradzacher]

and would cause a lot of unnecessary flagging in existing codebases

The rule has an autofixer for any cases that aren't any/unknown - so why is extra initial flagging bad?
It brings the codebase into a consistent style immediately. Anecdotally I've found people generally don't write return await so I don't think it would flag much.

[JoshuaKGoldberg]

people generally don't write return await

...but they should! If you return myPromise instead of return await myPromise the call stack doesn't include the intermediary step.

I'm +1 to this provided the caveats Kirk mentioned are accounted for.

[kirkwaiblinger]

so why is extra initial flagging bad?

Mostly, it's just that I would be scared we'd not get the rule into a recommended preset at all if we reawaken the opinion-charged conversations referenced in #8667 (comment). The thought is that, strategically speaking, we could sidestep all that and still get the most important benefits (IMO) of return-await.

It brings the codebase into a consistent style immediately. Anecdotally I've found people generally don't write return await so I don't think it would flag much.

I completely agree that full consistency (whether in-try-catch or, my preference, always) should be the correct end goal for any codebases. Maybe this could be in a strict or stylistic config?

[kirkwaiblinger]

If backlash that it's too opinionated where it doesn't affect the outcome of execution is not a concern, then everything I've said is moot! 🙂