Skip to content

vue/attributes-order: v-dom-portal errors if placed after v-if #422

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
ITOffcie opened this issue Mar 7, 2018 · 7 comments · Fixed by #421
Closed

vue/attributes-order: v-dom-portal errors if placed after v-if #422

ITOffcie opened this issue Mar 7, 2018 · 7 comments · Fixed by #421
Assignees
Milestone

Comments

@ITOffcie
Copy link

ITOffcie commented Mar 7, 2018

Tell us about your environment

  • ESLint Version: 4.14.0
  • eslint-plugin-vue Version: 4.3.0
  • Node Version: 8.9.4

Please show your full configuration:

"vue/attributes-order": [2, {
          order: [
              "DEFINITION",
              "LIST_RENDERING",
              "CONDITIONALS",
              "RENDER_MODIFIERS",
              "GLOBAL",
              "UNIQUE",
              "CONTENT",
              "OTHER_ATTR",
              "BINDING",
              "EVENTS"
          ]
      }]

What did you do? Please include the actual source code causing the issue.

<div v-if="isVisible" v-dom-portal="true" :class="classes"></div>

What did you expect to happen?
v-dom-portal directive should not fail linting, as it should be OTHER_ATTR imo

What actually happened? Please include the actual, raw output from ESLint.
Instead it seems to be recognised as either DEFINITION or LIST_RENDERING, and the following message displays:

Attribute "v-dom-portal" should go before "v-if". (vue/attributes-order)
@MarcosMdo
Copy link

Having similar issue after updating to eslint-plugin-vue 4.3.0.

@hoopyfroody
Copy link

Yup, same thing here. Seems to happen with any custom directive in the form v-xxxx

@ydfzgyj
Copy link
Contributor

ydfzgyj commented Mar 9, 2018

I think all custom directives should be recognised as OTHER_ATTR, and I have submitted a PR and wait for review.

@michalsnik
Copy link
Member

Yes, all custom directives should be recognised as OTHER_ATTR, but bindings do not count here @ydfzgyj

@michalsnik michalsnik added the bug label Mar 21, 2018
@michalsnik
Copy link
Member

michalsnik commented Mar 22, 2018

I'm giving it second thought, and maybe we should create another category for OTHER_DIRECTIVES? I wouldn't want to see them mixed with normal attributes and bindings, how about you?

@michalsnik michalsnik added this to the v5.0.0 milestone Mar 24, 2018
@skyrpex
Copy link

skyrpex commented Mar 28, 2018

What about letting us cherrypick the exact attributes? For example, I would like the v-t directive (from vue-i18n) to be grouped with v-text and v-html, while having the other directives in a different priority.

"vue/attributes-order": [2, {
    order: [
        "DEFINITION",
        "LIST_RENDERING",
        "CONDITIONALS",
        "RENDER_MODIFIERS",
        "GLOBAL",
        "UNIQUE",
        "BINDING",
        "OTHER_ATTR",
        "EVENTS",
        ["CONTENT", "v-t"],
    ],
}]

@mitar
Copy link

mitar commented May 1, 2018

I agree with @skyrpex, I also think we should put them all into a separate category, but translation ones should go into content.

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.

7 participants