Skip to content

no-duplicate-attributes should have css exceptions #112

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

Closed
nchutchind opened this issue Jul 28, 2017 · 8 comments · Fixed by #113
Closed

no-duplicate-attributes should have css exceptions #112

nchutchind opened this issue Jul 28, 2017 · 8 comments · Fixed by #113

Comments

@nchutchind
Copy link

  • ESLint Version: 4.3.0
  • eslint-plugin-vue Version: 3.8.0
  • Node Version: 8.1.4
<span class="label label-rounded" :class="statusColor"></span>
<span style="font-weight:bold;" :style="{ color: activeColor, fontSize: fontSize + 'px' }"></span>

Duplicate attribute 'class'. (vue/no-duplicate-attributes)
Duplicate attribute 'style'. (vue/no-duplicate-attributes)

Class and Style, specifically, should be allowed to be duplicated since they tend to have a mix of static and dynamic content. You often have structural styling that gets set in stone at development time, but want reactive styles or classes applied at runtime.

Perhaps no-duplicate-attributes should take an option for ignoring css attributes (class and style).

@nchutchind nchutchind changed the title no-duplicate-attributes should have exceptions no-duplicate-attributes should have css exceptions Jul 28, 2017
@armano2
Copy link
Contributor

armano2 commented Jul 28, 2017

@nchutchind nice catch
Vue is allowing for duplicates for style and class but not always:

This is ok:

  • <div style="" :style="">
  • <div class="" :class="">

This is not ok:

  • <div style="" style="">
  • <div :style="" :style="">
  • <div class="" class="">
  • <div :class="" :class="">

I think we should add setting to allow this.

@armano2
Copy link
Contributor

armano2 commented Jul 29, 2017

My proposal for schema is:

[
  {
    type: 'object',
    properties: {
      allowCoexistClass: {
        type: 'boolean' // default: true
      },
      allowCoexistStyle: {
        type: 'boolean' // default: true
      }
    },
    additionalProperties: false
  }
]

https://vuejs.org/v2/guide/class-and-style.html#Object-Syntax

@nchutchind
Copy link
Author

nchutchind commented Jul 31, 2017

I'm sorry for being a few days late here, but the only complaint I would have about your proposal is that coexist is one word, so the properties should be allowCoexistClass and allowCoexistStyle.

Otherwise, it looks great!

(apologies for pedantry)

@armano2
Copy link
Contributor

armano2 commented Jul 31, 2017

@nchutchind thanks, PR updated 🗡️

@michalsnik michalsnik added the bug label Aug 1, 2017
@michalsnik
Copy link
Member

I wonder if we actually need those extra settings.. In my opinion allowing coexistance of bindings and attributes for class and style could be always allowed. It's such a common use case.. What do you think @chrisvfritz @mysticatea ?

@mysticatea
Copy link
Member

Thank you for the report!

I had not noticed that class and :class can coexist.
Yes, this is a bug. I think that the rule should ignore class and style by default. I think "good to have" about the option which disallows duplicate of class and style.

@armano2
Copy link
Contributor

armano2 commented Aug 1, 2017

@mysticatea by default i will allow class and :class and leave option to disallows duplicates.
this is how i implemented this in #113

armano2 added a commit to armano2/eslint-plugin-vue that referenced this issue Aug 1, 2017
filipalacerda pushed a commit to filipalacerda/eslint-plugin-vue that referenced this issue 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)
@ajaymarathe
Copy link

@nchutchind nice catch
Vue is allowing for duplicates for style and class but not always:

This is ok:

  • <div style="" :style="">
  • <div class="" :class="">

This is not ok:

  • <div style="" style="">
  • <div :style="" :style="">
  • <div class="" class="">
  • <div :class="" :class="">

I think we should add setting to allow this.

It's working, Thanks. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants