fix(prefer-wait-for): only report when imported from TL #152
Conversation
| context | ||
| .getDeclaredVariables(importSpecifier) | ||
| .forEach(variable => | ||
| variable.references.forEach(reference => | ||
| reportWait(reference.identifier) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Is there a way we could make this cleaner? 👀
There was a problem hiding this comment.
perhaps
context
.getDeclaredVariables(importSpecifier)
.flatMap(variable => variable.references)
.forEach(({ identifier })=> reportWait(identifier))? But IMO it's not too much of a difference - I think this is fine
And I think flatMap does not work in node <= 10
There was a problem hiding this comment.
Indeed, we can't use flatMap in node 10
There was a problem hiding this comment.
Maybe we could make the code cleaner by extracting the first forEach callback to its own function? It's the only thing I can think of.
| code: ` | ||
| cy.wait(); | ||
| `, | ||
| }, |
There was a problem hiding this comment.
I think we should consider testing require scenarios as well
There was a problem hiding this comment.
It would be nice if we can have an additional test for it, indeed.
| code: ` | ||
| cy.wait(); | ||
| `, | ||
| }, |
There was a problem hiding this comment.
It would be nice if we can have an additional test for it, indeed.
|
This won't be just additional tests, it would also mean that we have to modify the rule. const { wait } = require("@testing-library/foo");
async () => {
await wait();
} |
|
Ah of course. Fair enough, I'll approve it then. |
|
🎉 This PR is included in version 3.2.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
@all-contributors please add @Miguelerja for bug |
|
I've put up a pull request to add @Miguelerja! 🎉 |
Closes #151