feat(rule): disallow unnecessary async function wrapper for single await call
#1863
Conversation
…wait` call Signed-off-by: hainenber <[email protected]>
G-Rath
left a comment
G-Rath
left a comment
There was a problem hiding this comment.
We can't change our configs without doing a new major, so please remove this from recommended
(On mobile so can't currently do a full review, but figured I'd this in in case you get to it before me 🙂)
Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
|
I've done removing this rule from |
…lity + rephrase error message Signed-off-by: hainenber <[email protected]>
|
All amended in newest commit :D |
Signed-off-by: hainenber <[email protected]>
G-Rath
left a comment
G-Rath
left a comment
There was a problem hiding this comment.
I think this is pretty much there, just got a few more tests I'd like - also, I'm wondering if a name like no-unneeded-async-expect-function might be a better name?
I think the use of "unneeded" is good because it indicates the rule is about an optimization that lets you have less code, rather than guarding against a particular bug or gotcha
|
|
||
| ## Rule details | ||
|
|
||
| This rule triggers a warning if a single `await` function call is wrapped by an |
There was a problem hiding this comment.
I think it would be better to say something like "This rule triggers a warning if expect is passed an async function that has a single await call", since that is what "unnecessary" means in this context
| }); | ||
|
|
||
| ruleTester.run('no-async-wrapper-for-expected-promise', rule, { | ||
| valid: [ |
There was a problem hiding this comment.
it would be good to have some tests that:
- pass a non-async function to
expect - pass a reference to another function which is async
- uses
awaitwithin theexpectcall (e.g.expect(await x())) - uses of other matchers, to show that we don't actually care if
rejectsorresolveis being used - use of
awaitwithin an array
| invalid: [ | ||
| { | ||
| code: dedent` | ||
| it('should be fix', async () => { |
There was a problem hiding this comment.
nit: these would be more correct as "should be fixed" 😅
| }) | ||
| `, | ||
| ], | ||
| invalid: [ |
There was a problem hiding this comment.
it'd be good if we could have a test with say a Promise.all, to help prove the fixer works in more complex situations
2 participants
Closes #1722
I've implemented said rule to lint for unnecessary
asyncwrapper for singleawaitcall inspired (or blatantly copied? :D) from @G-Rath's ESQuery selector in linked issue.This is also put into
recommendedoption as suggested. PTAL when you have time, folks!