Rule proposal: class-method-style

[zanminkian]

Description

We have 2 styles to define a method in class:

class Foo {
  methodStyle() {
    console.log("I am method style method");
  }
  propertyStyle1 = () => {
    console.log("I am property style method");
  };
  propertyStyle2 = function () {
    console.log("I am property style method");
  };
}

I recommend method style and disallow property style. Therefore, this rule class-method-style accept an option to specify the style, method (default) or property.

Fail

class Foo {
  propertyStyle1 = () => {
    console.log("I am property style method");
  };
}

class Foo {
  propertyStyle2 = function () {
    console.log("I am property style method");
  };
}

Pass

class Foo {
  methodStyle() {
    console.log("I am method style method");
  }
}

Proposed rule name

class-method-style

Additional Info

Related rule: https://typescript-eslint.io/rules/method-signature-style.
Related issue: #1737

[fregante]

Makes sense, as long as the method doesn't use this. The only problem that comes to mind is that:

1. User intentionally writes prop = () => this.doSomething() ✅
2. User changes it to prop = () => doSomething(), which triggers the rule ❌
3. Rule or user changes it to prop() {return doSomething()} ✅
4. User later uses this again prop() {return doSomething(this)} ✅⚠️

In the 4th step, everything looks fine, but the original intent of the method is lost.

I wonder the the rule should go both ways and:

• force prop() {} when it doesn't detect this
• force prop: () => {} when it detects this

[zanminkian]

After a few days of thinking, I prefer this rule not providing the option to select method style or property style. Using property style is a bad practice.

// Firstly,
// This style is horrible obviously. Ban it.
class Foo {
  propertyStyle = function () {
    console.log("I am property style method");
  };
}

// Secondly,
// User cannot use `this` keyword in this style. Users do not have to put it in a class.
// Just use static method or extract this method to be a function outside of class.
// See https://eslint.org/docs/latest/rules/class-methods-use-this
class Foo {
  propertyStyle = () => {
    console.log("I am property style method");
  };
}

// Thirdly,
// If this arrow function is only one line, maybe we can allow it
class Foo {
  propertyStyle = () => console.log("I am property style method");
}

To summary, I think this rule should not care about the this keyword in functions' body.

[fregante]

I think you're missing the point of the property style: they are pre-bound.

class A {
  value = 1;
  get = () => console.log(this.value)
}

const a = new A()
setTimeout(a.get, 1000)

This will log 1. If you use a method, it will log undefined

There are only a few practical reasons to prefer methods over properties:

• methods are faster to instantiate (if I remember correctly)
• methods can be bound to whatever the caller prefers

What you're describing are just stylistic preferences, but this has actual runtime differences.

[zanminkian]

@fregante You are right. I've never used property style methods. I forgot that we can use this in property style method body. I use NestJS a lot, but I seldom see property style methods.

But I still personally prefer this rule to be a stylistic rule, which is used to enforce user to use one method style. Maybe providing an option to select the style is necessary.