Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add await option for next-tick-style rule #2508

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+244 −5
Open

feat: add await option for next-tick-style rule #2508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16d025f
feat: add await option for next-tick-style rule
codiini Jul 11, 2024
9732ced
chore: reposition enums
codiini Jul 11, 2024
4664831
fixes
codiini Jul 22, 2024
1be0671
add check for expression statement
codiini Jul 23, 2024
9084c53
Merge branch 'master' into await/next-tick-style
codiini Jul 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions docs/rules/next-tick-style.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
pageClass: rule-details
sidebarDepth: 0
title: vue/next-tick-style
description: enforce Promise or callback style in `nextTick`
description: enforce Promise, Await or callback style in `nextTick`
since: v7.5.0
---

# vue/next-tick-style

> enforce Promise or callback style in `nextTick`
> enforce Promise, Await or callback style in `nextTick`

- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

Expand Down Expand Up @@ -52,13 +52,45 @@ Default is set to `promise`.

```json
{
"vue/next-tick-style": ["error", "promise" | "callback"]
"vue/next-tick-style": ["error", "promise" | "await" | "callback"]
}
```

- `"promise"` (default) ... requires using the promise version.
- `"await"` ... requires using the await syntax version.
- `"callback"` ... requires using the callback version. Use this if you use a Vue version below v2.1.0.

### `"await"`

<eslint-code-block fix :rules="{'vue/next-tick-style': ['error', 'await']}">

```vue
<script>
import { nextTick as nt } from 'vue';

export default {
async mounted() {
/* ✓ GOOD */
await nt(); callback();
await Vue.nextTick(); this.callback();
await this.$nextTick(); this.callback();

/* ✗ BAD */
nt().then(() => callback());
Vue.nextTick().then(() => callback());
this.$nextTick().then(() => callback());
nt(() => callback());
nt(callback);
Vue.nextTick(() => callback());
Vue.nextTick(callback);
this.$nextTick(() => callback());
this.$nextTick(callback);
}
}
</script>
```

</eslint-code-block>
### `"callback"`

<eslint-code-block fix :rules="{'vue/next-tick-style': ['error', 'callback']}">
Expand Down
91 changes: 89 additions & 2 deletions lib/rules/next-tick-style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,44 @@ function isAwaitedPromise(callExpression) {
)
}

/**
* @param {CallExpression} callExpression
* @returns {boolean}
*/

/**
* @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).trim()
}
Comment on lines +100 to +102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this branch.


return sourceCode.getText(callback.body)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function body can't exact directly if it has return

const foo = async () => {
	nextTick(() => {
		return;
	})

	return false
}

returns false

const foo = async () => {
	await nextTick(); {
		return;
	}

	return false
}

returns undefined

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will also broken if the callback has .id or used arguments inside.

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:
Expand Down Expand Up @@ -123,6 +150,66 @@ module.exports = {
return
}

if (preferredStyle === 'await') {
if (
callExpression.arguments.length > 0 ||
callExpression.parent.type !== 'AwaitExpression'
) {
context.report({
node,
messageId: 'useAwait',
fix(fixer) {
const sourceCode = context.getSourceCode()

// Handle callback to await conversion
if (callExpression.arguments.length === 1) {
if (callExpression.parent.type !== 'ExpressionStatement') {
return null
}

const [args] = callExpression.arguments
let callbackBody = null

callbackBody =
args.type === 'ArrowFunctionExpression' ||
(args.type === 'FunctionExpression' && !args.generator)
? extractCallbackBody(args, sourceCode)
Copy link
Contributor

@fisker fisker Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The 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)}()`
Copy link
Contributor

Choose a reason for hiding this comment

The 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 foo is truly, it will not be called


const nextTickCaller = sourceCode.getText(
callExpression.callee
)
return fixer.replaceText(
callExpression.parent,
`await ${nextTickCaller}();${callbackBody};`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need make sure the nextTick call is an expression statement, otherwise this will cause sytax errors.

if (foo) nextTick(...)

(() => nextTick(...))();

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fisker How would you suggest the fix method handle these cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore or turn into a block.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ignore" means no fix

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've added a check.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it related to this one: #2508 (comment) ?
I removed the .slice() method which keeps the block

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry didn't see that change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries. Is everything good to go now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a maintainer..

Copy link
Contributor

Choose a reason for hiding this comment

The 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 1 2

await nextTick();  console.log(2)
console.log(1)

Logs 2 1

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)
Expand Down
120 changes: 120 additions & 0 deletions tests/lib/rules/next-tick-style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@ tester.run('next-tick-style', rule, {
}</script>`,
options: ['promise']
},
{
filename: 'test.vue',
code: `<script>import { nextTick as nt } from 'vue';
export default {
async mounted() {
await this.$nextTick(); callback();
await Vue.nextTick(); callback();
await nt(); callback();
}
}</script>`,
options: ['await']
},
{
filename: 'test.vue',
code: `<script>import { nextTick as nt } from 'vue';
Expand Down Expand Up @@ -102,6 +114,22 @@ tester.run('next-tick-style', rule, {
foo.then(this.$nextTick, catchHandler);
}
}</script>`,
options: ['await']
},
{
filename: 'test.vue',
code: `<script>import { nextTick as nt } from 'vue';
export default {
mounted() {
foo.then(this.$nextTick);
foo.then(Vue.nextTick);
foo.then(nt);

foo.then(nt, catchHandler);
foo.then(Vue.nextTick, catchHandler);
foo.then(this.$nextTick, catchHandler);
}
}</script>`,
options: ['callback']
}
],
Expand Down Expand Up @@ -237,6 +265,98 @@ tester.run('next-tick-style', rule, {
}
]
},
{
filename: 'test.vue',
code: `<script>import { nextTick as nt } from 'vue';
export default {
async mounted() {
this.$nextTick(() => callback());
Vue.nextTick(() => callback());
nt(() => callback());

this.$nextTick(callback);
Vue.nextTick(callback);
nt(callback);

this.$nextTick().then(() => callback());
Vue.nextTick().then(() => callback());
nt().then(() => callback());
}
}</script>`,
output: `<script>import { nextTick as nt } from 'vue';
export default {
async mounted() {
await this.$nextTick();callback();
await Vue.nextTick();callback();
await nt();callback();

await this.$nextTick();callback();
await Vue.nextTick();callback();
await nt();callback();

await this.$nextTick();callback();
await Vue.nextTick();callback();
await nt();callback();
}
}</script>`,
options: ['await'],
errors: [
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 4,
column: 16
},
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 5,
column: 15
},
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 6,
column: 11
},
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 8,
column: 16
},
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 9,
column: 15
},
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 10,
column: 11
},
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 12,
column: 16
},
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 13,
column: 15
},
{
message:
'Use the await keyword with the Promise returned by `nextTick` instead of passing a callback function or using `.then()`.',
line: 14,
column: 11
}
]
},
{
filename: 'test.vue',
code: `<script>import { nextTick as nt } from 'vue';
Expand Down