Skip to content

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

Merged
merged 3 commits into from
Aug 3, 2017

Conversation

armano2
Copy link
Contributor

@armano2 armano2 commented Jul 29, 2017

allowCoexistClass - Enables v-bind:class directive can co-exist with the plain class attribute.
allowCoexistStyle - Enables v-bind:style directive can co-exist with the plain style attribute.

This PR should fix bug: fixes #112 .

@mysticatea
Copy link
Member

Thank you for contributing! I'm sorry for the delay.

Please see #112 (comment)

@armano2 armano2 force-pushed the patch-21-no-duplicate-attributes branch from de7e193 to 33631a3 Compare August 1, 2017 19:08
Copy link
Member

@mysticatea mysticatea left a 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?

return directiveNames.has(name)
}
if (allowCoexistClass && name === 'class') {
return directiveNames.has(name)
Copy link
Member

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.

Copy link
Contributor Author

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, 👍

const names = new Set()
const options = context.options[0] || {}
const allowCoexistStyle = Boolean(options.allowCoexistStyle !== false)
const allowCoexistClass = Boolean(options.allowCoexistClass !== false)
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, that's correct

@armano2
Copy link
Contributor Author

armano2 commented Aug 3, 2017

@mysticatea changes applied and i added few more tests

Copy link
Member

@mysticatea mysticatea left a 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!

@mysticatea mysticatea merged commit f834a7e into vuejs:master Aug 3, 2017
@armano2 armano2 deleted the patch-21-no-duplicate-attributes branch August 3, 2017 22:02
filipalacerda pushed a commit to filipalacerda/eslint-plugin-vue that referenced this pull request Aug 5, 2017
* 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)
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.

no-duplicate-attributes should have css exceptions
3 participants