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

The require-await rule does more harm than good #1220

Open
dbrockman opened this issue Jan 2, 2024 · 1 comment
Open

The require-await rule does more harm than good #1220

dbrockman opened this issue Jan 2, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@dbrockman
Copy link

The require-await rule tells the user to remove the async keyword when await is not used in the function. This is bad advise and will change the behaviour of the function. If you remove the async keyword then any error that is thrown in the function will no longer result in a rejected promise. A function that returns a Promise should always return a Promise, even in case of errors.

I think a better way to implement this rule would be to invert it. If the return type is Promise<...> then require the use of the async keyword.

Lint Name

require-await

Code Snippet

Simple example

// Linter tells me to remove the `async` keyword here
async function fail(): Promise<void> {
  throw new Error("Failed");
}

Slightly more realistic example

function assertString(value: unknown): asserts value is string {
  if (typeof value !== "string") {
    throw new Error("Expected a string");
  }
}

// Linter tells me to remove async here
async function sha256(input: unknown): Promise<ArrayBuffer | null> {
  // Assert that input is a string
  assertString(input);

  // Call a method that would fail if the input is not a string
  const utf8 = new TextEncoder().encode(input);

  // Return the result of an async function without await
  // It would be better if the linter told me to add an extra await here instead of removing async.
  return globalThis.crypto.subtle.digest("SHA-256", utf8);
}

async function main() {
  const hash = await sha256(123).catch(() => null);
  console.log(hash);
}

Expected Result

The linter should not warn about the async keyword.

Actual Result

The lister tells the user to remove the async keyword.

If you follow the advice of the linter and remove the async keywords, you will get an uncaught error.

Additional Info

Version

deno 1.39.1 (release, x86_64-apple-darwin)
v8 12.0.267.8
typescript 5.3.3

@dbrockman dbrockman added the bug Something isn't working label Jan 2, 2024
@karfau
Copy link

karfau commented May 6, 2024

Similar but with a different proposed solution: #1170

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants