Skip to content

Extend vue/padding-line-between-tags to allow conditional tags to be placed next to each other #2215

Open
@LoiLock

Description

@LoiLock

What rule do you want to change?
I suggest a new option for vue/padding-line-between-tags to allow conditional tags to be placed together.

Currently, the following doesn't work with the default settings:
image

This rule has been my favorite addition to eslint-plugin-vue, but our team prefers grouping conditional tags together, since it's easy to identify that there's a conditional element just by looking whether there's a newline between them or not.

Activity

LoiLock

LoiLock commented on Jun 16, 2023

@LoiLock
Author

I'll probably take a look at submitting a pr myself, but I don't know what the best name for the config option would be. Any suggestions?

FloEdelmann

FloEdelmann commented on Jun 16, 2023

@FloEdelmann
Member

Maybe, to build upon the current syntax (which looks like CSS selectors), something like this? But it's quite verbose for a possibly common use case.

{
  "vue/padding-line-between-tags": ["error", [
    { "blankLine": "always", "prev": "*", "next": "*" },
    { "blankLine": "never", "prev": "[v-if]", "next": "[v-else]" },
    { "blankLine": "never", "prev": "[v-if]", "next": "[v-else-if]" },
    { "blankLine": "never", "prev": "[v-else-if]", "next": "[v-else-if]" },
    { "blankLine": "never", "prev": "[v-else-if]", "next": "[v-else]" }
  ]]
}

Or it could be shortened by allowing selector lists:

{
  "vue/padding-line-between-tags": ["error", [
    { "blankLine": "always", "prev": "*", "next": "*" },
    { "blankLine": "never", "prev": "[v-if], [v-else-if]", "next": "[v-else-if], [v-else]" }
  ]]
}
LoiLock

LoiLock commented on Jun 16, 2023

@LoiLock
Author

I was thinking of excludeConditional, very simple and straight to the point, or perhaps add non-conditional to blankLine?

LoiLock

LoiLock commented on Jun 16, 2023

@LoiLock
Author

Maybe, to build upon the current syntax (which looks like CSS selectors), something like this? But it's quite verbose for a possibly common use case.

{
  "vue/padding-line-between-tags": ["error", [
    { "blankLine": "always", "prev": "*", "next": "*" },
    { "blankLine": "never", "prev": "[v-if]", "next": "[v-else]" },
    { "blankLine": "never", "prev": "[v-if]", "next": "[v-else-if]" },
    { "blankLine": "never", "prev": "[v-else-if]", "next": "[v-else-if]" },
    { "blankLine": "never", "prev": "[v-else-if]", "next": "[v-else]" }
  ]]
}

Or it could be shortened by allowing selector lists:

{
  "vue/padding-line-between-tags": ["error", [
    { "blankLine": "always", "prev": "*", "next": "*" },
    { "blankLine": "never", "prev": "[v-if], [v-else-if]", "next": "[v-else-if], [v-else]" }
  ]]
}

The only thing I don't like about this is how it quickly becomes quite messy to catch all conditional blocks.

Looking at it now, perhaps conditional with the options always and never would be more consistent?

FloEdelmann

FloEdelmann commented on Jun 16, 2023

@FloEdelmann
Member

excludeConditional has the problem that you can't enforce no blank lines between conditional tags.

"non-conditional" option in blankLine has the problem that you can't use it together with consistent.

conditional with the options always and never sounds wrong, since you're actually controlling the blank line between conditional tags.

Maybe { "blankLine": "never", "betweenConditionalTags": true }?

LoiLock

LoiLock commented on Jun 16, 2023

@LoiLock
Author

I'll go with { "blankLine": "never", "betweenConditionalTags": true }

LoiLock

LoiLock commented on Jun 16, 2023

@LoiLock
Author

@FloEdelmann I've made some changes. Could you (if you have time, of course) check it out and give me some feedback? I'm missing quite a few tests, and I see that there's a ton of tests for various edge cases, and I'm not sure for which scenarios to test.

I don't want to pollute the pull requests with broken code. As far as I know, everything works so far, the existing tests are all still working. I've made sure it works with consistent, and it should work with a variety of edge cases.

Sorry, it's my first time working on a linter rule 😅

https://github.com/LoiLock/eslint-plugin-vue

FloEdelmann

FloEdelmann commented on Jun 22, 2023

@FloEdelmann
Member

I had a quick look at the test cases (at master...LoiLock:eslint-plugin-vue:master).

Could you please change the first two added valid cases like this, to make it more clear what happens:

{ blankLine: 'always', prev: '*', next: '*', betweenConditionalTags: false }

      <template>
        <div>
          <div v-if="true === true">Foo</div>
-         <div v-else>bar</div>
+         <div v-else-if="bar">bar</div>
+
+         <div v-else>baz</div>
+
+         <p>unconditional</p>
        </div>
      </template>

(your third test case can then be deleted, as this one has the same options)

{ blankLine: 'always', prev: '*', next: '*', betweenConditionalTags: true }

      <template>
        <div>
          <div v-if="true === true">Foo</div>

          <div v-else>bar</div>

-         <div>baz</div
+         <div>baz</div>
+         <p>unconditional</p>
+
+         <p>another unconditional element</p>
        </div>
      </template>

Also, could you please add more invalid test cases and more valid test cases with never and consistent?

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @FloEdelmann@LoiLock

        Issue actions

          Extend `vue/padding-line-between-tags` to allow conditional tags to be placed next to each other · Issue #2215 · vuejs/eslint-plugin-vue