-
-
Notifications
You must be signed in to change notification settings - Fork 681
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
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 |
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:class
directive can co-exist with the plainclass
attribute.allowCoexistStyle
- Enablesv-bind:style
directive can co-exist with the plainstyle
attribute.This PR should fix bug: fixes #112 .