feat: new rule await-async-utils #69
Conversation
| @@ -24,7 +24,7 @@ | |||
|
|
|||
| <!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section --> | |||
|
|
|||
| [](#contributors) | |||
| [](#contributors-) | |||
There was a problem hiding this comment.
I removed this by mistake in that PR trying to fix the badges.
| @@ -0,0 +1,103 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
This rule implementation is almost the same as await-async-query but I didn't try to optimize this as we may migrate this to TS soon.
| testingLibraryUtilUsage.push(node); | ||
| } | ||
| }, | ||
| 'Program:exit'() { |
There was a problem hiding this comment.
Ah interesting, I didn’t know about this
There was a problem hiding this comment.
Me either! I learnt about it thanks to @thomlom, you can find more info here
lib/rules/await-async-utils.js
Outdated
| function isPromiseResolved(node) { | ||
| const parent = node.parent; | ||
|
|
||
| const hasAThenProperty = node => |
There was a problem hiding this comment.
Does this need to be declared inside the isPromiseResolved function? Just to avoid confusion with arg names.
There was a problem hiding this comment.
Not really, let me move this outside.
|
Quick question: why not merging |
Well, even if the implementation looks similar (but it's not the same 100%), the async stuff being checked is different so as we already had a rule for async queries, we would have different rule for async utils to 1) being able to config what you want to lint and 2) not mix different concerns (ones are queries to get elements, others are utils to wait until something happen). I think mixing both in the same rule would be a mistake because 1) the name of the rule wouldn't make sense with what is checking and 2) we could rename the rule and include everything but then every single async function from testing library should go under that rule and it seems too much. What I think we should do is merge proper utils these 2 rules implementation have in common, but I commented previously I didn't do that assuming we will migrate this to TS soon so we can abstract this better. |
|
Merging as @thomlom approved outside github (he's busy dealing with personal stuff) and @emmenko approved before my last little change. |
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #49