refactor: add no-return-await lint rule #4384
Conversation
There was a problem hiding this comment.
LGTM, awesome...
When await is not used in the method/function block, does async cause another microtask to be scheduled? Either way, if you are returning a promise in all branches (like a lot of these updates) the method no longer needs to be async. There is an eslint rule of require-await which would catch this as well. With TypeScript checking the return type it is very unlikely enforcing this would cause incorrect behaviour.
Not sure, but thanks for the lead.
I'll see to it tomorrow |
Hmm, isn't it a good indicator for a function that returns a Promise to be marked async? Or does it affect performance? (I guess I disagree with "either way".) EDIT: |
It is an internal concern, it has no external detectable difference... When you generate a type definition for example, there is no indication that the method is async or not. |
| } | ||
|
|
||
| /** Writes a fixed HTTP response to the socket rid. Returns bytes written. */ | ||
| async function write(rid, data) { | ||
| return await sendAsync(ops["write"], rid, data); | ||
| return sendAsync(ops["write"], rid, data); |
There was a problem hiding this comment.
It will be interesting to see if this has any effect.
There was a problem hiding this comment.
It should. It is at least one less microtask, unless V8 optimises it out, which it might be smart enough to do.
@kitsonk I did a quick check and we'd have to add a lot of supressions to tests (because there's a lot of mocks that assert async functions were called). I'm open to that change, but I'd open a new issue for that. |
https://eslint.org/docs/rules/no-return-await