Rule Change: lines-between-class-members customize properties/instance field spacing

[domdomegg]

What rule do you want to change?

lines-between-class-members

What change to do you want to make?

Generate fewer warnings

How do you think the change should be implemented?

A new option

Example code

// ["error", { "properties": "off", "methods": "always" }]
class ClassA {
  fieldA = 'Field A'
  fieldB = this.fieldA

  fieldC = 42

  instanceMethodA() {
    return 'abc'
  }

  instanceMethodB() {
    return 'def'
  }
}

// ["error", { "properties": "never", "methods": "always" }]
class ClassB {
  fieldA = 'Field A'
  fieldB = this.fieldA
  fieldC = 42

  instanceMethodA() {
    return 'abc'
  }

  instanceMethodB() {
    return 'def'
  }
}

// ["error", { "properties": "always", "methods": "never" }]
class ClassB {
  fieldA = 'Field A'

  fieldB = this.fieldA

  fieldC = 42

  instanceMethodA() {
    return 'abc'
  }
  instanceMethodB() {
    return 'def'
  }
}

What does the rule currently do for this code?

Either have to have spaces between everything, or spaces between nothing (with some changes if you enable exceptAfterSingleLine: but this seems to be used as a proxy rather than actual control - and can't enforce that there shouldn't be lines between single line members).

What will the rule do after it's changed?

As per example code.

Participation

• I am willing to submit a pull request to implement this change.

Additional comments

This was previously discussed in #13295, where it was agreed this would be discussed when the syntax reached stage 4, which it now has.

Not sure if this is now out of scope with the stylistic rule changes, but would argue that given this is a feature that has moved to stage 4 since last review it may fall under 'New ECMAScript features'.

[Rec0iL99]

Hi @domdomegg, thanks for the issue.

I can confirm the proposal has reached stage 4.

cc @eslint/eslint-tsc

[nzakas]

Thanks for the suggestion. Can you please explain what option you're proposing to add and what it would change? "As per example" code doesn't actually tell us what you're proposing. We need a description to really understand what it is you're proposing.

[domdomegg]

Current state

Currently, lines-between-class-members either forces lines between every class member, or forces no lines between every class member.

So you have to pick between:

class ClassWithInstanceField {
  fieldA = 'Field A'

  fieldB = this.fieldA

  fieldC = 42

  methodA() {
    return 'abc'
  }

  methodB() {
    return 'def'
  }
}

or

class ClassWithInstanceField {
  fieldA = 'Field A'
  fieldB = this.fieldA
  fieldC = 42
  methodA() {
    return 'abc'
  }
  methodB() {
    return 'def'
  }
}

According to #13295 (comment), this was because at the time members were non-stage 4 syntax and therefore considered out of scope for ESLint.

Proposed state

Options to set spacing for property and and method members independently.

So the option type would be something like:

type Option = "always" | "never" | { properties: "always" | "never" | "off", methods: "always" | "never" | "off" }

Allowing for:

// ["error", { "properties": "off", "methods": "always" }]
class ClassA {
  fieldA = 'Field A'
  fieldB = this.fieldA

  fieldC = 42

  instanceMethodA() {
    return 'abc'
  }

  instanceMethodB() {
    return 'def'
  }
}

// ["error", { "properties": "never", "methods": "always" }]
class ClassB {
  fieldA = 'Field A'
  fieldB = this.fieldA
  fieldC = 42

  instanceMethodA() {
    return 'abc'
  }

  instanceMethodB() {
    return 'def'
  }
}

// ["error", { "properties": "always", "methods": "never" }]
class ClassB {
  fieldA = 'Field A'

  fieldB = this.fieldA

  fieldC = 42

  instanceMethodA() {
    return 'abc'
  }
  instanceMethodB() {
    return 'def'
  }
}

Why

This allows enforcing consistent spacing between methods (as this is standard), while enabling users to group properties (as this is useful for readability). This appears to be a common desire, as indicated by #13295 and typescript-eslint/typescript-eslint#977.

This also allows for differences between properties and methods. For example, at my organisation we have a code standard that currently requires a custom ESLint rule to enforce no spacing between properties, and spacing between methods.

[nzakas]

Got it, thanks for explaining. I think this makes sense, though I'd suggest replacing "properties" with "fields", as they are described in the spec. (Properties are foo.bar format.)

type Option = "always" | "never" | { fields: "always" | "never" | "off", methods: "always" | "never" | "off" }

@mdjermanovic what do you think?

[mdjermanovic]

I agree that "fields" would be better than "properties".

Getters and setters will fall under "methods"?

[mdjermanovic]

How will it be determined from the configuration whether or not there should be a blank line between a field and a method?

class C {
  fieldA = 'Field A'  
  instanceMethodA() {
    return 'abc'
  }
}

[domdomegg]

I'd suggest replacing "properties" with "fields"
I agree that "fields" would be better than "properties".

Sounds good, also agreed this makes sense.

Getters and setters will fall under "methods"?

Yes, I think so.

How will it be determined from the configuration whether or not there should be a blank line between a field and a method?

I gave this some thought. I think in the majority of cases it makes most sense to put in a newline if either has a setting for a newline (e.g. in your example if either of fields or methods is true, insert a newline, otherwise don't). I think this makes sense given that if you are introducing spacing between a particular type of entry, it would be odd for it to be bunched up against other things: that makes the visual spacing of a class look unbalanced to me, e.g. this looks wrong:

// this looks bad to me
class C {
  fieldA = 'Field A'

  fieldB = 'Field B'
  instanceMethodA() {
    return 'abc'
  }
}

and so does:

// this looks bad to me
class C {
  fieldA = 'Field A'
  instanceMethodA() {
    return 'abc'
  }

  instanceMethodB() {
    return 'abc'
  }
}

The other reasonable alternative is to pick one of them, probably the later clause, and use the rule for that (e.g. in your example because the method follows the field, we use the setting for method).

[nzakas]

@mdjermanovic can you check back on this?

And if you agree we should make a change, let's mark as accepted.

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

[nzakas]

@mdjermanovic still looking for your response on this.

[mdjermanovic]

Sorry for the delay.

I think the logic would be clearer with a schema like the one in padding-line-between-statements.

{
    rules: {
        "lines-between-class-members": ["error", {
            // requires blank lines around methods, disallows blank lines between fields
            enforce: [
                { blankLine: "always", prev: "*", next: "method" },
                { blankLine: "always", prev: "method", next: "*" },
                { blankLine: "never", prev: "field", next: "field" }
            ]
        }]
    }
}

[dko-slapdash]

@mdjermanovic Super-demanded feature!

As people say, "better is the enemy of the good". The syntax change similar to padding-line-between-statements would likely be hard to implement, whilst fields/methods property is straightforward and covers 99.9% of the usecases for people (people really don't want empty lines between properties and do want them between methods, it may even be the default).

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

[Rec0iL99]

Not stale.

I guess we can mark this as accepted? How do we proceed further?

cc @nzakas @mdjermanovic

[nzakas]

@mdjermanovic because you were driving this, up to you to mark as accepted.

[mdjermanovic]

Accepted, PR is welcome.

I think the schema I suggested in #17082 (comment) is easier to understand and implement because there will be no ambiguity about how the rule should work.

[snitin315]

I'll work on this