feat: add support for resolves/rejects matchers in async queries #64
Conversation
There was a problem hiding this comment.
Awesome! Just one small thing: can you add some examples using resolves and rejects to await-async-query valid examples within its doc?
Btw those util methods you added for checking this look like could be extracted in the future for checking proper resolves/rejects in other rules, so let's keep an eye on that in case we can reuse it in the future 💪
| @@ -101,3 +105,18 @@ function isPromiseResolved(node) { | |||
| // promise.then(...) | |||
| return hasAThenProperty(parent); | |||
| } | |||
|
|
|||
| function hasClosestExpectResolvesRejects(node) { | |||
| if (!node.parent) { | |||
There was a problem hiding this comment.
Is this necessary? Because on L34 the function is called with isAwaited(node.parent.parent), which implies that node.parent cannot be undefined.
There was a problem hiding this comment.
Good catch. I initially wrote a utility function called findClosestExpectCallthat was supposed to be reusable. At that time the node.parent thing was necessary because the function was recursive. Then I refactored it and indeed, it's not necessary.
There was a problem hiding this comment.
I tried to remove the node.parent check and test broke which is, in the end, logical. While isAwaited(node.parent.parent) implies node.parent cannot be undefined, it's very possible that hasClosestExpectResolvesRejects goes beyond node.parent.parent (until the Program node) in case you're not using an expect statement.
I'll add the examples 😉 About the util methods, we could look into converting the codebase to typescript, especially this package: typescript-eslint/experimental-utils! |
I would also be in favor of that, as it makes working with the AST much easier and safer with the type declarations. |
|
Can we merge it then @Belco90 ? |
|
Sure! On it, sorry. |
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I think I'll create a project within this github repo so we can discuss which things we need to address to migrate the plugin to TS and split proper work. |
Closes #51