-
-
Notifications
You must be signed in to change notification settings - Fork 664
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
feat: add await option for next-tick-style rule #2508
base: master
Are you sure you want to change the base?
Changes from 2 commits
16d025f
9732ced
4664831
1be0671
9084c53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -79,17 +79,53 @@ function isAwaitedPromise(callExpression) { | |||||
) | ||||||
} | ||||||
|
||||||
/** | ||||||
* @param {CallExpression} callExpression | ||||||
* @returns {boolean} | ||||||
*/ | ||||||
function isAwaitedFunction(callExpression) { | ||||||
return ( | ||||||
callExpression.parent.type === 'AwaitExpression' && | ||||||
callExpression.parent.parent.type !== 'MemberExpression' | ||||||
) | ||||||
} | ||||||
|
||||||
/** | ||||||
* @param {Expression | SpreadElement} callback | ||||||
* @param {SourceCode} sourceCode | ||||||
* @returns {string} | ||||||
*/ | ||||||
function extractCallbackBody(callback, sourceCode) { | ||||||
if ( | ||||||
callback.type !== 'FunctionExpression' && | ||||||
callback.type !== 'ArrowFunctionExpression' | ||||||
) { | ||||||
return '' | ||||||
} | ||||||
|
||||||
if (callback.body.type === 'BlockStatement') { | ||||||
return sourceCode | ||||||
.getText(callback.body) | ||||||
.slice(1, -1) // Remove curly braces | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can simply keep the block. |
||||||
.trim() | ||||||
} | ||||||
|
||||||
return sourceCode.getText(callback.body) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The function body can't exact directly if it has const foo = async () => {
nextTick(() => {
return;
})
return false
} returns const foo = async () => {
await nextTick(); {
return;
}
return false
} returns There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will also broken if the callback has const foo = async () => {
nextTick(function foo() {
use(foo)
use(arguments)
})
} |
||||||
} | ||||||
|
||||||
module.exports = { | ||||||
meta: { | ||||||
type: 'suggestion', | ||||||
docs: { | ||||||
description: 'enforce Promise or callback style in `nextTick`', | ||||||
description: 'enforce Promise, Await or callback style in `nextTick`', | ||||||
categories: undefined, | ||||||
url: 'https://eslint.vuejs.org/rules/next-tick-style.html' | ||||||
}, | ||||||
fixable: 'code', | ||||||
schema: [{ enum: ['promise', 'callback'] }], | ||||||
schema: [{ enum: ['promise', 'await', 'callback'] }], | ||||||
messages: { | ||||||
useAwait: | ||||||
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.', | ||||||
usePromise: | ||||||
'Use the Promise returned by `nextTick` instead of passing a callback function.', | ||||||
useCallback: | ||||||
|
@@ -123,6 +159,61 @@ module.exports = { | |||||
return | ||||||
} | ||||||
|
||||||
if (preferredStyle === 'await') { | ||||||
if ( | ||||||
callExpression.arguments.length > 0 || | ||||||
!isAwaitedFunction(callExpression) | ||||||
) { | ||||||
context.report({ | ||||||
node, | ||||||
messageId: 'useAwait', | ||||||
fix(fixer) { | ||||||
const sourceCode = context.getSourceCode() | ||||||
|
||||||
// Handle callback to await conversion | ||||||
if (callExpression.arguments.length > 0) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
const [args] = callExpression.arguments | ||||||
let callbackBody = null | ||||||
|
||||||
callbackBody = | ||||||
args.type === 'ArrowFunctionExpression' || | ||||||
args.type === 'FunctionExpression' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
? extractCallbackBody(args, sourceCode) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't extract callback body directly, there might be variable conflicts. export default {
async created() {
const foo = 1;
nextTick(() => {
const foo = 2;
doSomething(foo);
})
}
} |
||||||
: `${sourceCode.getText(args)}()` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also not safe nextTick(foo || bar) - await nextTick(); foo || bar()
+ await nextTick(); (foo || bar)() If |
||||||
|
||||||
const nextTickCaller = sourceCode.getText( | ||||||
callExpression.callee | ||||||
) | ||||||
return fixer.replaceText( | ||||||
callExpression.parent, | ||||||
`await ${nextTickCaller}();${callbackBody};` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need make sure the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fisker How would you suggest the fix method handle these cases? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ignore or turn into a block. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "ignore" means no fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I've added a check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't it related to this one: #2508 (comment) ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry didn't see that change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No worries. Is everything good to go now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a maintainer.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems not the correct fix.. It changes behavior nextTick(() => console.log(2))
console.log(1) Logs await nextTick(); console.log(2)
console.log(1) Logs Maybe use suggestions instead? |
||||||
) | ||||||
} | ||||||
|
||||||
// Handle promise to await conversion | ||||||
if (isAwaitedPromise(callExpression)) { | ||||||
const thenCall = callExpression.parent.parent | ||||||
if (thenCall === null || thenCall.type !== 'CallExpression') | ||||||
return null | ||||||
const [thenCallback] = thenCall.arguments | ||||||
if (thenCallback) { | ||||||
const thenCallbackBody = extractCallbackBody( | ||||||
thenCallback, | ||||||
sourceCode | ||||||
) | ||||||
return fixer.replaceText( | ||||||
thenCall, | ||||||
`await ${sourceCode.getText(callExpression)};${thenCallbackBody}` | ||||||
) | ||||||
} | ||||||
} | ||||||
return null | ||||||
} | ||||||
}) | ||||||
} | ||||||
|
||||||
return | ||||||
} | ||||||
if ( | ||||||
callExpression.arguments.length > 0 || | ||||||
!isAwaitedPromise(callExpression) | ||||||
|
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.
Why this
.parent.parent
check needed.