feat(valid-expect): supporting automatically fixing missing await in some cases
#1574
Conversation
| @@ -3,6 +3,9 @@ | |||
| 💼 This rule is enabled in the ✅ `recommended` | |||
| [config](https://github.com/jest-community/eslint-plugin-jest/blob/main/README.md#shareable-configurations). | |||
|
|
|||
| 🔧 This rule is automatically fixable by the | |||
There was a problem hiding this comment.
can we have it say that "some patterns are fixable" or something like that? specifically now, only if the function is already async
There was a problem hiding this comment.
I attempted to fix it with commit 90028bf. What do you think?
There was a problem hiding this comment.
fwiw it seems currently the docs generator doesn't support customizing this in anyway, so I think what you've done is fine for now - @bmish might be something worth adding; even just the ability to explicitly name particular rules as "partially fixable" in the config which would case this string to include with "in some cases" suffix
G-Rath
changed the title
await in some cases
There was a problem hiding this comment.
This looks like a good start to me, though noting it is by no means complete so I've updated the title to reflect that and removed the "resolving" of #1140
I think though it's fine to land as-is since it's an improvement regardless 🎉
docs/rules/valid-expect.md
Outdated
| Note: Test function will be fixed if it is async and does not have await in the | ||
| async assertion. |
There was a problem hiding this comment.
we could use fancy alerts here
There was a problem hiding this comment.
nice! I made the fix in commit ee54e6a.
# [28.4.0](v28.3.0...v28.4.0) (2024-05-03) ### Features * **valid-expect:** supporting automatically fixing missing `await` in some cases ([#1574](#1574)) ([a407098](a407098))
|
🎉 This PR is included in version 28.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
| const findFirstAsyncFunction = ({ | ||
| parent, | ||
| }: TSESTree.Node): TSESTree.Node | null => { | ||
| if (!parent) { | ||
| return null; | ||
| } | ||
|
|
||
| return isFunction(parent) && parent.async | ||
| ? parent | ||
| : findFirstAsyncFunction(parent); | ||
| }; |
There was a problem hiding this comment.
Sorry for the late response; I only saw this after the new release.
Doesn't this cause false-positives for non-async functions contained in async functions? It's probably a really rare situation in test files, but I think it would be more correct like this:
if (!parent) {
return null;
}
if (isFunction(parent)) {
return parent.async ? parent : null;
}
return findFirstAsyncFunction(parent);There was a problem hiding this comment.
Ah, I see the logic is changed in #1579 anyway.
There was a problem hiding this comment.
Do you have a test case that's failing? If so please share either way so we can ensure that going forward it doesn't fail
There was a problem hiding this comment.
No failing test case, just noticed this while reading the code.
When
awaitis missing , it's addingawait.Relates to #1140