Skip to content

⭐️New: Add vue/no-use-v-if-with-v-for rule #406

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

Merged
merged 23 commits into from
Jul 11, 2018

Conversation

ota-meshi
Copy link
Member

This PR adds vue/no-use-v-if-with-v-for rule.
This implements rule proposed in #403

michalsnik and others added 18 commits January 28, 2018 23:55
* [Update] Make `vue/max-attributes-per-line` fixable

* [fix] bug and style

* [fix] Switch indent calculation method with node and attribute

* [fix] don't handle indentation

* [add] autofix test max-attributes-per-line.js
* [Update] Make `vue/order-in-components` fixable

This Commit makes `vue/order-in-components` fixable.
In case of `The "A" property should be above the "B" property` error, autofix will move A before B

* [fix] fail test at [email protected]

* [fix] If there is a possibility of a side effect, don't autofix

* [fix] failed test at node v4

* [update] use Traverser

* [fix] failed test [email protected]

* [fix] I used `output: null` to specify "not fix"
}]
```

:+1: Examples of additional **correct** code for this rule with sample `"allowUsingIterationVar": true` options:
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is supposed to be incorrect right?

/>
```

:-1: Examples of additional **incorrect** code for this rule with sample `"allowUsingIterationVar": true` options:
Copy link
Member

Choose a reason for hiding this comment

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

And this correct

context.report({
node,
loc: node.loc,
message: "The 'v-for' list variable should be replace with a new computed property that returns your filtered list by this 'v-if' condition."
Copy link
Member

Choose a reason for hiding this comment

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

Can we make list to be dynamic value in this message? Also what do you think about this kind of message?

The 'someArray' variable inside 'v-for' directive should be replaced with a computed property that returns filtered array instead. You should not mix 'v-for' with 'v-if'.

@michalsnik michalsnik self-assigned this Mar 22, 2018
@ota-meshi
Copy link
Member Author

@michalsnik Thank you for review!
I changed it, so please check it.

@michalsnik michalsnik changed the title [New] Add vue/no-use-v-if-with-v-for rule ⭐️New: Add vue/no-use-v-if-with-v-for rule Apr 1, 2018
Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

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

Good job @ota-meshi !

@michalsnik michalsnik merged commit a95dfbb into vuejs:master Jul 11, 2018
@ota-meshi ota-meshi deleted the add-no-use-v-if-with-v-for branch July 11, 2018 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants