-
Notifications
You must be signed in to change notification settings - Fork 248
feat(rule): disallow unnecessary async function wrapper for single await call
#1863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(rule): disallow unnecessary async function wrapper for single await call
#1863
Conversation
…wait` call Signed-off-by: hainenber <dotronghai96@gmail.com>
G-Rath
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <dotronghai96@gmail.com>
Signed-off-by: hainenber <dotronghai96@gmail.com>
|
I've done removing this rule from |
| dedent` | ||
| it('pass', async () => { | ||
| expect(); | ||
| }) | ||
| `, | ||
| dedent` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you clean up the indentation of the code samples? the code should be indented one level from the indent level of the dedent, and the end backtick should be at the same indent as the dedent
i.e.
| dedent` | |
| it('pass', async () => { | |
| expect(); | |
| }) | |
| `, | |
| dedent` | |
| dedent` | |
| it('pass', async () => { | |
| expect(); | |
| }) | |
| `, | |
| dedent` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. I'll amend in next commit.
| (awaitNode?.type !== AST_NODE_TYPES.ArrowFunctionExpression && | ||
| awaitNode?.type !== AST_NODE_TYPES.FunctionExpression) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we've got a utility for this
| (awaitNode?.type !== AST_NODE_TYPES.ArrowFunctionExpression && | |
| awaitNode?.type !== AST_NODE_TYPES.FunctionExpression) || | |
| !awaitNode || | |
| !isFunction(awaitNode) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL. I'll amend in next commit
| fixable: 'code', | ||
| messages: { | ||
| noAsyncWrapperForExpectedPromise: | ||
| 'Rejected/resolved promises should not be wrapped in async function', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of a phrasing more like "Unneeded async function"? that way we don't have to deal with the "rejected/resolved", and I think it's a bit more positive to be conveying we think you can have less code as an optimization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm cool with that 👍
…lity + rephrase error message Signed-off-by: hainenber <dotronghai96@gmail.com>
|
All amended in newest commit :D |
Signed-off-by: hainenber <dotronghai96@gmail.com>
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!