deno_lint icon indicating copy to clipboard operation
deno_lint copied to clipboard

no-this-alias is a bit noisy

Open callionica opened this issue 3 years ago • 4 comments

It's common to want to assign this to a variable when implementing tree methods and there's no harm in it (as below). Shouldn't this rule only fire if the alias is used in a local function or escapes the current method?

get root(): this {
        // deno-lint-ignore no-this-alias
        let primary = this;
        while (true) {
            if (primary.parent !== undefined) {
                primary = primary.parent;
            } else {
                return primary;
            }
        }
    }

callionica avatar Oct 03 '20 06:10 callionica

CC @magurotuna what do you think?

bartlomieju avatar Aug 24 '21 17:08 bartlomieju

Yes, indeed there's no harm with the this alias in the snippet. But in this particular case, there's a way to achieve the same goal without storing this in a variable. I mean, if I were to rewrite the logic, I would do:

get root(): this {
  if (this.parent === undefined) {
    return this;
  }
  return this.parent.root;
}

I think this looks much clearer and simpler.

If there are cases where we definitely need to store this in a variable to do something, then I think we should fix the logic of no-this-alias rule, but I don't come up with those cases right now.

magurotuna avatar Aug 30 '21 14:08 magurotuna

Yes, indeed there's no harm with the this alias in the snippet. But in this particular case, there's a way to achieve the same goal without storing this in a variable. I mean, if I were to rewrite the logic, I would do:

get root(): this {
  if (this.parent === undefined) {
    return this;
  }
  return this.parent.root;
}

I think this looks much clearer and simpler.

If there are cases where we definitely need to store this in a variable to do something, then I think we should fix the logic of no-this-alias rule, but I don't come up with those cases right now.

Any updates? I have a class and in a method where i use cheerio to grab some data, i have a property say name. And to have access to the right value for this.name within a cheerio element map ($(element).map(function(){ ... })) inside a method, i have to first assign this to a variable(eg: const parent = this) and inside the loop, i can have access to the value by using parent.name. If i don't do that, i get the wrong value for this.name.

I have disabled the warning.

waptik avatar Aug 31 '22 09:08 waptik

@waptik You should use arrow functions. See: https://typescript-eslint.io/rules/no-this-alias/

Nikaple avatar Oct 26 '22 09:10 Nikaple