Typescript: Better error and quick fix for missing await

Created on 29 Mar 2019  Â·  7Comments  Â·  Source: microsoft/TypeScript

Search Terms

missing await error refactoring

Suggestion

Missing an await can cause some confusion especially for people who haven't used async/await before. A helpful error message when you have a Promise\

  • Using operators, and T is a primitive type. If T were say an object type, it seems more likely something else is wrong with the code to me.
  • Passing to a function, and T is a valid parameter at that position.
  • for-of and T is some array type
  • The target of a member expression and T has the member (or maybe the property name is not in Promise).

A quick fix could also be considered. When in an async function and an error such as above appears, a quick fix that adds an await before the source of the promise within that function.

Use Cases

It helps people realize when they need an await.

Checklist

My suggestion meets these guidelines:

  • [x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • [x] This wouldn't change the runtime behavior of existing JavaScript code
  • [x] This could be implemented without emitting different JS based on the types of the expressions
  • [x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • [x] This feature would agree with the rest of TypeScript's Design Goals.
Error Messages Experience Enhancement Suggestion

Most helpful comment

Another suggestion related to this one is the opposite: When there's "await" on things that are not Promises.

const calculateSum = (a: number, b: number) => a + b;

async function someAsyncFn() {
     // ...
    const result = await calculateSum(a, b);
    // ...
}

Personally I would prefer to see a warning (or an error) on the const result = await calculateSum(a, b); telling the user that the await is unnecessary since TypeScript already knows, by inference, that the outcome of that call is synchronous and there is nothing to await. That would push the user to cleanup his code and remove the unnecessary async/await all together.

All 7 comments

Ugh, realized html ate a <T> above. The request should hopefully be more clear now.

It's funny because I filed #22923 but not this I guess?

Another suggestion related to this one is the opposite: When there's "await" on things that are not Promises.

const calculateSum = (a: number, b: number) => a + b;

async function someAsyncFn() {
     // ...
    const result = await calculateSum(a, b);
    // ...
}

Those are definitely not errors, but I think it helps on readability if synchronous stuff is kept in a synchronous way.
Keep in mind that in case calculateSum returns number | Promise<number> then this warning shouldn't show up.

I'm really not sure if this can/should be added into this issue - For one part it feels closely related (using or not using await on promises) but on the other hand the original issue is tackling something that can potentially lead to errors whereas this one is just.... style.

Yes, now I'm more convinced this shouldn't be here.

@voliva Since most of the legwork should already be done at that point i see no reson to not add this.

Another suggestion related to this one is the opposite: When there's "await" on things that are not Promises.

const calculateSum = (a: number, b: number) => a + b;

async function someAsyncFn() {
     // ...
    const result = await calculateSum(a, b);
    // ...
}

Personally I would prefer to see a warning (or an error) on the const result = await calculateSum(a, b); telling the user that the await is unnecessary since TypeScript already knows, by inference, that the outcome of that call is synchronous and there is nothing to await. That would push the user to cleanup his code and remove the unnecessary async/await all together.

Would a missing-await from an expression-statement also be in-scope? For example:

foo(); // foo: () => Promise<void>

Or is that a whole different class of detectable problems?

@aslatter it’s perfectly well detectable, but unlike the other cases mentioned here, it’s not an error. It’s possibly something we could add a suggestion diagnostic for (like unnecessary await at #32363), but I think it’s even less clear than the unnecessary await case that it’s a mistake. It could very well be a “fire and forget” Promise for something that needn’t interfere with the primary control flow path of the program and doesn’t resolve to anything that you care to observe, e.g. firing a telemetry request or setting something non-critical in a Redis cache. You should probably still have a .catch in cases like that to avoid unhandled rejections, but I believe I’ve seen linter rules that look for that kind of thing exactly.

That said, it is extremely easy to introduce hard-to-find bugs by writing exactly that when you _did_ mean to await it, so I think a suggestion might be nice.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

blendsdk picture blendsdk  Â·  3Comments

bgrieder picture bgrieder  Â·  3Comments

Antony-Jones picture Antony-Jones  Â·  3Comments

fwanicka picture fwanicka  Â·  3Comments

CyrusNajmabadi picture CyrusNajmabadi  Â·  3Comments