-
-
Notifications
You must be signed in to change notification settings - Fork 699
Add new options for no-duplicate-attributes #112
#113
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
Add new options for no-duplicate-attributes #112
#113
Conversation
|
Thank you for contributing! I'm sorry for the delay. Please see #112 (comment) |
de7e193 to
33631a3
Compare
mysticatea
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.
Thank you for contributing!
Could you add a few tests?
lib/rules/no-duplicate-attributes.js
Outdated
| return directiveNames.has(name) | ||
| } | ||
| if (allowCoexistClass && name === 'class') { | ||
| return directiveNames.has(name) |
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'd like to see the test of the following case:
<div :class="a" class="b"></div>I guess that the class="b" is warned even if allowCoexistClass is true.
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.
@mysticatea you are completely right on this, 👍
lib/rules/no-duplicate-attributes.js
Outdated
| const names = new Set() | ||
| const options = context.options[0] || {} | ||
| const allowCoexistStyle = Boolean(options.allowCoexistStyle !== false) | ||
| const allowCoexistClass = Boolean(options.allowCoexistClass !== false) |
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 think that this cast is redundant since the result of !== expression is always boolean.
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.
yup, that's correct
|
@mysticatea changes applied and i added few more tests |
mysticatea
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.
LGTM, thank you very much!
* master: Add rule `vue/require-valid-default-prop`. (vuejs#119) 3.10.0 Update readme to 3.10.0 Chore: remove package-lock.json (vuejs#128) Fix: parserService must exist always (fixes vuejs#125) (vuejs#127) Add rule `require-render-return`. (vuejs#114) 3.9.0 Update package-lock Update: options for `no-duplicate-attributes` (fixes vuejs#112)(vuejs#113) New: add rule `attribute-hyphenation`. (fixes vuejs#92)(vuejs#95) Add namespace check of svg & mathML instead of tag names (vuejs#120)⚠️ Add support for deprecated state in update-rules⚠️ (vuejs#121) Add rules: `no-dupe-keys` and `no-reserved-keys`. (vuejs#88) Chore: Improve tests for name-property-casing & improve documentation (vuejs#115) New: add `require-prop-types` rule (fixes vuejs#19)(vuejs#85) Update: upgrade vue-eslint-parser (fixes vuejs#36, fixes vuejs#56, fixes vuejs#96) (vuejs#116)
allowCoexistClass- Enablesv-bind:classdirective can co-exist with the plainclassattribute.allowCoexistStyle- Enablesv-bind:styledirective can co-exist with the plainstyleattribute.This PR should fix bug: fixes #112 .