Rule proposal: `padding-line-between-component-options`

[barthy-koeln]

Please describe what the rule should do:
Enforces empty lines between top-level options of Vue components.

What category should the rule belong to?

• Enforces code style (layout)
• Warns about a potential error (problem)
• Suggests an alternate way of doing something (suggestion)
• Other (please specify:)

Provide 2-3 code examples that this rule should warn about:

//✓ GOOD
export default {
  name: 'a-button',

  data() {
    return {
      // ...
    }
  },

  computed: {
   // …
  }
}

// ✗ BAD
export default {
  name: 'a-button',
  data() {
    return {
      // ...
    }
  },
  computed: {
   // …
  }
}

Additional context

• Consistent format makes it easier to read, find, select, and extend.
• The rule can enforce no empty lines between options (swap GOOD and BAD in the example).
• I'm not sure about the term "options" but decided so because of the "Options API" name.
• StackOverflow Question requesting this (5 Upvotes)
• Github Comment requesting this (6 upvotes)

[ota-meshi]

Thank you for your proposal.
The new rule seems to have duplicate check contents with the new-line-between-multi-line-property rule.
https://eslint.vuejs.org/rules/new-line-between-multi-line-property.html

Do you have any ideas to clarify those roles?

[barthy-koeln]

@ota-meshi

This suggested rule focuses solely on 'top-level' options (i.e. data, props, computed, ...), while the new-line-between-multi-line-property rule applies to any nested object.

My team and I style components with padding lines in top-level, but not nested properties:

export default {
  name: 'QIcon',

  props: {
    value: {
      type: String,
      required: true
    },
    focused: {
      type: Boolean,
      default: false,
      required: true
    },
    label: String,
    icon: String
  },

  computed: {
    that() {
      return true
    },
    these () {
      return false
    }
  }
}

I think there is room for both behaviours, but this rule's behaviour could be toggled by an option (e.g. depth, topLevelOnly, ...) within new-line-between-multi-line-property.

Finally, the other rule's naming might be ambiguous, as I did not interpret "new-line" to be the same as an "empty line" or "padding line". I did search for existing similar rules but missed new-line-between-multi-line-property because of its name.

---

How would you suggest we proceed?

[ota-meshi]

@barthy-koeln

I think we need to deprecate the new-line-between-multi-line-property rule and add new rules to meet various style requirements.

The new rule should be able to set the always or never of padding for each scope (e.g. options, properties, all?).
For always, there is an additional option to apply only to multi-line properties.
I don't have an idea for a rule name.
What do you think?

cc @FloEdelmann

---

Finally, the other rule's naming might be ambiguous, as I did not interpret "new-line" to be the same as an "empty line" or "padding line". I did search for existing similar rules but missed new-line-between-multi-line-property because of its name.

Since there is no rule naming convention, the rule name is determined by the rule proposer, implementer, and reviewer when the rule is implemented. So the rule names may not be unified. However, it is difficult to make a breaking change to rename after adding a rule.

[FloEdelmann]

Ah, I reckoned there was a similar rule, but couldn't find it either.

I also support to deprecate new-line-between-multi-line-property and add a new rule with options for different scopes. My suggestion:

• Rule name: vue/padding-lines-in-component-definition
• Options:

  • Either object form:

    "vue/padding-lines-in-component-definition": {
        // non-negative integer defining the number of empty lines between e.g. `name`, `data`, `computed`
        // "ignore" to not check the top level
        "topLevel": 0 | 1 | 2 | … | "ignore",
    
        // non-negative integer defining the number of empty lines between e.g. individual prop definitions, method definitions
        // "ignore" to not check the second level
        "secondLevel": 0 | 1 | 2 | … | "ignore",
    
        // non-negative integer defining the number of empty lines between e.g. computed getter/setter, watch options
        // "ignore" to not check the third level
        "thirdLevel": 0 | 1 | 2 | … | "ignore",
    
        // whether to treat multiple consecutive single-line properties (e.g. `name`, `inheritAttrs`) as a single block
        // and expect no padding lines between them
        "groupSingleLineProperties": true | false
    }

  • Or integer form:

    "vue/padding-lines-in-component-definition": 0 | 1 | 2 | … // shorthand that sets all `topLevel`, `secondLevel` and `thirdLevel`

  • Default:

    {
        "topLevel": 1,
        "secondLevel": 1,
        "thirdLevel": 1,
        "groupSingleLineProperties": true
    }

[ota-meshi]

We cannot add an option to specify the number of rows in the empty lines. Forcing more than one empty line conflicts with ESLint's core rule no-multiple-empty-lines.

https://eslint.org/docs/rules/no-multiple-empty-lines

The rule should only check if it has empty lines.
Therefore, I think it is better to make always, never, and ignore as options.

---

I don't think the name secondLevel is good.
I think watch and coupled may use a third level.

{
  computed: {
    myProp: {
      get () {
        // ...
      },

      set (newValue) {
        // ...
      }
    }
  },
  watch: {
    myProp: {
      handler (val, oldVal) {
        // ...
      },

      deep: true
    },
  }
}

[ota-meshi]

I noticed how I had to consider whether to check defineProps({}) etc.
Do you think the new rule will check for objects in defineProps({}) etc?
I think the name topLebelcan be confusing to the user if the rule checks it.
I think the name component-definition can be confusing to the user if the rule doesn't check it.

[FloEdelmann]

We cannot add an option to specify the number of rows in the empty lines.

That suggestion was based on the current options of new-line-between-multi-line-property, where we already allow specifying that.
What about projects where no-multiple-empty-lines is disabled and there should be more padding between top-level options than between second level options?

Therefore, I think it is better to make always, never, and ignore as options.

If we decide to not check for a specific number, these names are good. I also updated my comment to change null to "ignore".

I don't think the name secondLevel is good.

Unfortunately, I couldn't come up with a better name. What do you suggest instead?

I think watch and coupled may use a third level.

Ah, indeed. I added it to my comment.

[ota-meshi]

That suggestion was based on the current options of new-line-between-multi-line-property, where we already allow specifying that.

The number specified in the options of the new-line-between-multi-line-property rule is the minimum number of lines to be judged as multiple line property. It's not the number of empty lines.

Unfortunately, I couldn't come up with a better name. What do you suggest instead?

I haven't formed an opinion yet, but what about using the name options instead of the name topLevel?
I think the secondLevel and thirdLevel can be set to one, but I haven't come up with an idea for it good name yet.

[ota-meshi]

I think the secondLevel and thirdLevel can be set to one, but I haven't come up with an idea for it good name yet.

What about the name declarations?
I think that defines may be better. This option can also be applied to defineEmits() and defineProps().

[FloEdelmann]

The number specified in the options of the new-line-between-multi-line-property rule is the minimum number of lines to be judged as multiple property. It's not the number of empty lines.

Ah, you're right.

defineProps({})

Hmm, good point. Maybe splitting the rule up into separate rules would make sense? That'd also avoid having to find better names for topLevel / secondLevel / etc.

Nevermind, I changed my mind during writing this comment. This suggestion gets too messy.

Details

Each rule could then have one string option ("always" | "multiline-only" | "never").

• padding-line-between-component-options: looks inside export default {…}, defineComponent({…})
• padding-line-between-props: looks inside export default { props: {…} }, defineComponent({ props: {…} }), defineProps({…})
• padding-line-between-emits: looks inside export default { emits: {…} }, defineComponent({ emits: {…} }), defineEmits({…})
• padding-line-between-computed-properties: looks inside export default { computed: {…} }, defineComponent({ computed: {…} })
• padding-line-between-methods: looks inside export default { methods: {…} }, defineComponent({ methods: {…} })
• padding-line-between-computed-getter-setter: looks inside export default { computed: { foo: {…} } }, defineComponent({ computed: { foo: {…} } })
• padding-line-between-watch-options: looks inside export default { watch: { foo: {…} } }, defineComponent({ watch: { foo: {…} } })

[barthy-koeln]

Since vue/padding-lines-in-component-definition would affect all objects within a component definition, I would suggest a "default" or "global" setting, overridden for specific keys:

"vue/padding-lines-in-component-definition": {
    "padding": "always" | "never" | "ignore"
    
    // defaults to value of "padding"
    "options": "always" | "never" | "ignore"

    // defaults to value of "padding"
    "computed": "always" | "never" | "ignore"

    // ...
}