-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Better error and quick fix for missing await #30646
Comments
Ugh, realized html ate a |
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. 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. |
Personally I would prefer to see a warning (or an error) on the |
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 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. |
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<T> instead of T that suggests maybe an await is missing might help address this. There are at least a few common cases this comes up: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:
The text was updated successfully, but these errors were encountered: