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

Remove unnecessary return await #5117

Merged
merged 2 commits into from
Feb 13, 2023
Merged

Conversation

nflaig
Copy link
Member

@nflaig nflaig commented Feb 8, 2023

Motivation

from eslint docs no-return-await

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

Using return await inside an async function keeps the current function in the call stack until the Promise that is being awaited has resolved, at the cost of an extra microtask before resolving the outer Promise

Benefit

Improve performance by avoiding the extra microtask

Trade off

The function no longer being a part of the stack trace if an error is thrown asynchronously from the Promise being returned. This can make debugging more difficult.

I have personally not experienced any debugging issues due to missing await in a return statement, if a developers wants a piece of code to be more debuggable should be explicit about it by assigning the return value to a variable before returning.

async function foo() {
    const x = await bar();
    return x;
}

Description

  • Add eslint rule to disallow unnecessary return await
  • Fix unnecessary return await statements

@nflaig nflaig requested a review from a team as a code owner February 8, 2023 10:58
@nflaig nflaig changed the title Disallow unnecessary return await Remove unnecessary return await Feb 8, 2023
@wemeetagain
Copy link
Member

@dapplion should review this since he had some opinions on this in the past

@dapplion
Copy link
Contributor

As long as you don't do

try {
  await foo() 
} catch () {}

all those return await are useless

@wemeetagain wemeetagain reopened this Feb 13, 2023
@wemeetagain
Copy link
Member

reopened to get CI to restart

@wemeetagain wemeetagain enabled auto-merge (squash) February 13, 2023 18:28
@wemeetagain wemeetagain merged commit 4cec93e into ChainSafe:unstable Feb 13, 2023
@wemeetagain
Copy link
Member

🎉 This PR is included in v1.6.0 🎉

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

Successfully merging this pull request may close these issues.

3 participants