[no-invalid-this] incorrectly flagging class properties

[vladfrangu]

Hello!

I've recently started migrating my projects from tslint to eslint (so far, successfully!!) but I've encountered one small issue with the no-invalid-this rule provided by eslint.

Specifically, in a case similar to:

	private readonly YES_NO_USAGE = new Usage(this.message.client as KlasaClient, '<yesno:boolean>', '');

	constructor(private readonly message: KlasaMessage) {
		this.EMOJI_USAGE.customizeResponse('emoji', () => `You provided an emoji I either don't know about, or is not valid unicode.. Please provide another one`);
	}

it will error due to the this being "invalid" (while the compiled code is totally valid).

Is this a case of creating a specific @typescript-eslint-plugin rule for no-invalid-this?

Thanks!

[bradzacher]

Yes, this is because ESLint has no concept of class properties as they are only just (or not quite yet?) finalised.

Either we'd have to wait for ESLint to implement it into the base rule, or create our own version of the rule within this plugin.

I'd lean toward the former, as they should soon move to stage 4 (i'd hope) https://github.com/tc39/proposal-class-fields

From the ESLint readme:

Once a language feature has been adopted into the ECMAScript standard (stage 4 according to the TC39 process), we will accept issues and pull requests related to the new feature...

If it looks like it's going to be a while, we can consider bringing the rule in.

[fabb]

It's unfortunate, because TypeScript allows to use features of stage >= 3.

[bradzacher]

Happy to accept a PR with an extension rule if people don't want to wait for the official rule to be updated!

[TehShrike]

This issue is tagged with "good first issue" – as someone who hasn't written an eslint rule before, how would an eslint-plugin noob get started? Forking eslint's no-invalid-this plugin and making some change to take advantage of the typescript parser?

[bradzacher]

there are a number of examples of extension rules in the codebase already.
There are two options for extending a base rule, both of which require you to read + understand the original rule's source code first.

The first option is the preferred option.
You can create a new rule, and import the base rule. In your new rule, you call the base rule's selectors when appropriate.
Examples:

• https://github.com/typescript-eslint/typescript-eslint/blob/aeea4cd40e2ac66ff596a205ba7bfc95aa376128/packages/eslint-plugin/src/rules/camelcase.ts
• https://github.com/typescript-eslint/typescript-eslint/blob/aeea4cd40e2ac66ff596a205ba7bfc95aa376128/packages/eslint-plugin/src/rules/no-empty-function.ts
• https://github.com/typescript-eslint/typescript-eslint/blob/aeea4cd40e2ac66ff596a205ba7bfc95aa376128/packages/eslint-plugin/src/rules/no-extra-parens.ts
• https://github.com/typescript-eslint/typescript-eslint/blob/aeea4cd40e2ac66ff596a205ba7bfc95aa376128/packages/eslint-plugin/src/rules/no-magic-numbers.ts
• https://github.com/typescript-eslint/typescript-eslint/blob/aeea4cd40e2ac66ff596a205ba7bfc95aa376128/packages/eslint-plugin/src/rules/require-await.ts
• https://github.com/typescript-eslint/typescript-eslint/blob/aeea4cd40e2ac66ff596a205ba7bfc95aa376128/packages/eslint-plugin/src/rules/quotes.ts
• https://github.com/typescript-eslint/typescript-eslint/blob/aeea4cd40e2ac66ff596a205ba7bfc95aa376128/packages/eslint-plugin/src/rules/no-useless-constructor.ts

This approach is preferred because it reuses the existing code, which means we automatically get any fixes or improvements that they put into the source code.

---

The second approach is just copying the code from the base rule, and modifying it to suit the typescript use case.
This approach is less preferred for obvious reasons - it should only be used if you need access to internal logic / data structures that you can't get via just guarding some selectors.

[snebjorn]

@bradzacher may I suggest https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/ROADMAP.md be updated. It's currently listed as

🌟 = in ESLint core

it should probably be

🌓 = implementations differ or ESLint version is missing functionality

On another note. If this is implemented in typescript-eslint it would be helpful if function usage in class methods wouldn't just say

Unexpected 'this'. eslint(no-invalid-this)

But state that it's not allowed as it's bound to the function and not the class. Or something like that :)

[Ky6uk]

That kind of code also reported incorrectly now. But I'm unsure this is incorrect behavior actually because this still function inside another function. But I think this is related to the issue.

class Dog {
  private debouncedBork = debounce(() => {
    this.bork();
//  ^^^^ Unexpected 'this'.eslint@typescript-eslint/no-invalid-this
  });
}

[bradzacher]

Please don't "bump" issues. It just creates notification spam and annoys everyone subscribed to the thread.

We are a community maintained project - we rely upon contributions from the community to help move the project forward.
If this is important to you, consider submitting a PR.
If you can't submit a PR; that's perfectly fine, but please wait patiently until another community member is able to work on this.

[krisztianb]

Just to give everyone an update here. I created a PR #2685 which should fix the issues reported here.