Disable the class-methods-use-this rule

[leroykorterink]

There is a proposal to disable the class-methods-use-this rule.

class-methods-use-this: this rule introduces unnecessary congnitive complexity. Sometimes the code is cleaner and easier to read when all methods are defined in the class, even if they don't use this.
- @edgardz

Please see the documentation

[leroykorterink]

I don't think we should disable this rule, there are some clear benefits to using normal functions over methods when the this keyword is not used in a method.

• An unused method cannot be tree-shaked by tools like Webpack/Vite.
• A function is easier to re-use when it's not a method.
• Non-static methods are harder to test because an instance of the class must be created before it can be invoked.

I do agree that in some cases it's easier to reason about methods when they're part of a class. In that scenario it's fine to disable the rule for these specific methods.

[ThaNarie]

In our current (very large) project we have only about 10 disable occurrences, which fall into two categories:

• getter functions that use external imported utils/code (global state)
• overridden function of extended class that only use parameters, or "static" data

In those cases it's still useful that these methods belong as instance methods:

• as the code that's calling it might not know about on which static class it's defined, it just has an object passed down to call the method on
• as the implementation of these methods could change, and it's not worth to change surrounding code from this to static every time that happens.

So in general I do think that this rule forces best practises when working with classes, and exceptions are limited.

But I'm happy to be proven wrong with some code examples of cases where classes might be used differently, and this becomes a bigger issue.

[leroykorterink]

I can think of some standard library like classes where I'd prefer static methods over functions, but it's fine to use an eslint-disable comment in these specific cases.

I'd prefer this

class Vector2 {
  public add(v: Vector2): Vector2 {
    ...
  }
}

Vector2.normalize(myVector);

over this

function add(v1: Vector2, v2: Vector2): Vector2 {
  ...
}

normalizeVector2(myVector);

[ThaNarie]

Both are unrelated to the rule, since it doesn't use instance methods, but rather static methods.

[edgardz]

This rule causes the second method of this class to fail because it doesn't use the word this internally:

class Service {
  public getFormattedInput = (input: string): string => {
    const formatted = this.formatInputToSomeInternalClassNeeds(input)
    return formatted
  }

  private formatInputToSomeInternalClassNeeds = (input: string): string => {
    const result = `formatted-${input}`
    return result
  }
}

Should this method be really outside the class since it will only be used inside it?

[ThaNarie]

Not outside, but static perhaps?

class Service {
  public getFormattedInput = (input: string): string => {
    const formatted = Service.formatInputToSomeInternalClassNeeds(input)
    return formatted
  }

  private static formatInputToSomeInternalClassNeeds(input: string): string {
    const result = `formatted-${input}`
    return result
  }
}

[leroykorterink]

Ideally it should be outside the class because it cannot be tree shaked if the method is added as a static method to a class. This means that the code will be part of the bundle when it becomes unused and it is not removed from the source.

This can be a bit annoying in some cases because it can be very convenient when a function is encapsulated in a class like the example in this comment. This approach shouldn't be the default for function that aren't used too often because of the tree shaking problem.