Deprecate no-unused-variable rule for typescript@next; also fix runtime error

[jkillian]

PR checklist

• Addresses an existing issue: Fix breaking typescript@next tests #3914, no-unused-variable TypeError: Cannot read property '1' of null #3918
• bugfix
  • Includes tests

Overview of change:

The latest versions of TS 2.9-nightly change how they report some diagnostics, this PR makes sure the rule doesn't throw given those changes.

However, the changes in 2.9 make it much more complex for this rule to work as intended, especially with respect to the ignorePattern option. As per the discussion in #3918, we henceforth deprecate the rule for TS 2.9 and later, since the compiler handles most of what this rules does adequately.

CHANGELOG.md entry:

[deprecation] no-unused-variable is deprecated because typescript now covers most of its functionality

[suchanlee]

it looks like @next is still broken :(

[suchanlee]

you can also use semver to test versions

[giladgray]

yes prefer using semver

[jkillian]

good call, udpated

[kachkaev]

It is unfortunate that this option has been deprecated again. Here is at least one scenario when defining no-unused-variable is very useful: #4046 (comment)

Could this option be undeprecated again like in #2256?

[eamodio]

Yeah, why was this re-deprecated? The issues with the built-in typescript flags haven't changed (afaik) -- they are still compile errors, where as no-unused-variable can be a warning.

[adidahiya]

Please see the discussion in #3918 (it's linked right at the top of the PR).

[eamodio]

Unfortunately this leaves things in a poor state (and I very much get that the rule was/is causing to much complexity/issues), because typescript isn't going to make those compiler options warnings, and from there issues about it have recommended using TSLint for that instead. 😢

[atsu85]

@kachkaev wrote:

It is unfortunate that this option has been deprecated again. Here is at least one scenario when defining no-unused-variable is very useful: #4046 (comment)

Another use-case where i really need it, is generated code. I want my manually written code to follow this rule, but to avoid making code-generator enormously complex, i'd like to disable this rule in generated code (or at least in some sections of generated code) - that is smth that can't be done with TypeScript compiler option.

Please undeprecate this rule again! 😢

[dolanmiu]

Another reason why I need it

In Angular, sometimes you need to have to use @HostBinding, which can be private. I haven't found a way to get around it yet, apart from to disable tslint for the line:

// tslint:disable-next-line:no-unused-variable
@HostBinding('class') private classes = 'my-theme';

https://stackoverflow.com/questions/52594965/no-unused-variable-tslint-rule-does-not-work-with-private-hostbinding

[BartoszWasiluk]

Another reason why I need it

In Angular, sometimes you need to have to use @HostBinding, which can be private. I haven't found a way to get around it yet, apart from to disable tslint for the line:

// tslint:disable-next-line:no-unused-variable
@HostBinding('class') private classes = 'my-theme';

https://stackoverflow.com/questions/52594965/no-unused-variable-tslint-rule-does-not-work-with-private-hostbinding

IMO Inputs, Outputs, HostBindings etc. shouldn't be private, because angular uses them outside of class
I don't understand why would you declare them as private.

[dolanmiu]

IMO Inputs, Outputs, HostBindings etc. shouldn't be private, because angular uses them outside of class
I don't understand why would you declare them as private.

Because sometimes @HostBinding is not used in the corresponding template or outside, so therefore they are private.

I do not see why you should break encapsulation for the sake of it

@Input and @Ouput I agree should be public because:

<app-my-component [hello]="foo" (myOutput)="bar"></app-my-component>

[BartoszWasiluk]

Because sometimes @HostBinding is not used in the corresponding template or outside, so therefore they are private.

I do not see why you should break encapsulation for the sake of it

@Input and @Ouput I agree should be public because:

<app-my-component [hello]="foo" (myOutput)="bar"></app-my-component>

But it's the same way you are using your public variables in a template.

How do you think Angular would use your @HostBinding?

I think it's something like that -> nativeElement.setAttribute('class', _component.classes);
ofc it's compiled to js and those types doesn't really matter but according to the semantics it should be public

Your variable is never read privately in class, so it has to be read outside of your component.

[andyfleming]

Really disappointed to see this deprecation. Having a compilation failure is very annoying during active development. Linting is specifically where I want to catch this issue.

[dolanmiu]

@BartoszWasiluk

So say you have an @Input in a @Component, which is only used within the class, would you still make that Input public? Or would you make it private (it's only used in the class, not in the template):

e.g.

@Component({
  selector: 'app-my-component',
  templateUrl: './my.component.html',
  styleUrls: ['./my.component.scss']
})
export class MyComponent {
  @Input() private readonly label: string; // not used in template

  ...

}

[ikokostya]

Totally agree with #3919 (comment) and #3919 (comment) and vote for undeprecate this rule back.

See my explanation in buzinas/tslint-eslint-rules#171 (comment)

[thomas-darling]

Yeah, would you please bring this back - treating unused variables as build errors during active development is extremely annoying when commenting out code just to test something.
We absolutely need this rule.

[ikokostya]

I think discussion was moved to this issue #4100

[BartoszWasiluk]

@dolanmiu
Read this => https://www.typescriptlang.org/docs/handbook/decorators.html
And think of how Angular could implement Angular Input decorator.
@Input('myInput') test: number;
<my-component [myInput]="10"></my-component>
Decorator gets property name or decorator param if provided
and binds it with property name
kinda inputsObj = { myInput: 'test' };
So if your binding to [myInput] changes it uses that binding
binding = { input: 'myInput', value: 10 };
componentInstance[inputsObj[binding.input]] = binding.value;
Of course it's way more complicated but my example shows that it HAS to access componentInstance property publicly.
That link might be also helpful to understand the concept:
https://blog.angularindepth.com/implementing-custom-component-decorator-in-angular-4d037d5a3f0d
In my opinion it just proofs that you DON'T NEED unused private because it SHOULDN'T be private.