Typescript: Decorated private propoerty flagged as unused with `noUnusedLocals:true`

[kgtkr]

noUnusedLocals:true

import { Component } from '@angular/core';

@Component({
  selector: 'my-component',
  template: `
    {{text}}
  `,
})
export class MyComponent {
  @Input()
  private text: string;//'text' is declared but never used
}

[mhegazy]

So the proposal is to allow decorated properties to go unflagged?

[andrui]

I believe that this is almost impossible to predict all template usage (if we are talking not only about decorated properties/functions). E.g. in one project we use webpack and fucntion require('...') to include template into decorator metadata

@Component({
  selector: 'my-component',
  template: require('path/to/template'),
})

Sure, requested feature (detect usage in templates) would be very nice, but it requries integration with angular's compiler like it is done for react I guess...

[andrui]

The problem is really annoying for angular projects. Especially for re-usable components lib when some properties/methods should be private, but they are called only from component's template. This causes "unused local" error for such a methods/variables. So, if there is no possibility to check templates then the best solution is to introduce some "ignore" comment to avoid compiler errors in such a cases.

[aluanhaddad]

@andrui Just remember that if you ever try to use the angular AOT compiler, which is essentially a subset of typescript, all of your private fields used in template bindings will cause compilation errors and you have to rewrite your code.

This is off topic, since it isn't typescript at all at that point, but I thought I'd mention it since it seems to nullify the use case for many users.

[HerringtonDarkholme]

Some more background:
https://github.com/mgechev/codelyzer/issues/212

Using private member in template is explicitly listed as anti-pattern. A template system will use a class' member as public when hydrating template code. So technically a member in template should not be annotated as private.

[awerlang]

It is flagged only because it is private? What does it happen when it is declared as public?

In case it doesn't flag, there's the case for @Output() members, that noUnusedLocals should report if there's no reference to it. There's a bit of coding style attached to this, but still a good task for a compiler, instead of a linter.

Sure, requested feature (detect usage in templates) would be very nice, but it requries integration with angular's compiler like it is done for react I guess...

The ngc compiler would be able to do it. First it generates .ts files from .html templates, then it feeds everything to tsc to parse, compile and generate .js files.

[mhegazy]

It is flagged only because it is private? What does it happen when it is declared as public?

public properties on a class or an interface, and exporteded members on a module, are part of your public API, it does not make much sense to check if they are used or not, since they may be intended only for users of your library, and will never be used in the library itself.

private properties, as well as non-exported declarations, are not visible outside the scope, and thus cannot be used. if they are not used inside the class/module, the compiler can certify that they will never be used.

now, for the decorator scenario, this is something that is happening outside the knowledge of the compiler. it can not attest to its correctness or validity. for these cases, my recommendation is to switch off --noUnusedLocals.

[tomdale]

I ran into a related issue that I think highlights the problem without the private annotation muddying the waters.

In Glimmer, we have some test suite helpers that rely on classes and class decorators. E.g. https://github.com/glimmerjs/glimmer-vm/blob/master/packages/@glimmer/runtime/test/rendering/content-test.ts#L10.

Simplified example:

import {
  VersionedObject,
  strip,
  template,
  testModule,
  RenderingTest
} from "@glimmer/test-helpers";

@testModule('Static Content Tests')
class StaticContentTests extends RenderingTest {
  @template(`Hello World!`)
  ['renders text']() {
    this.render({});
    this.assertContent(`Hello World!`);
    this.assertStableRerender();
  }
}

This example triggers the noUnusedLocals warning for the entire class, even though the imported class decorator ultimately adds it to a global test harness.

Given that:

1. Semantically, class decorators are just functions that are passed the class.
2. If I imported a function and passed the class to that function, I would not get the noUnusedLocals error.

It seems to follow that class decorators should be considered "use" from the perspective of noUnusedLocals.

[aluanhaddad]

@tomdale a simplified repro

(function () {
  @decorated class A {}

  function decorated(t: new (...args: Array<{}>) => object) {};
}());

[justinhoward]

Using https://github.com/pana-cc/mocha-typescript with its decorators also triggers noUnusedLocals.

[aluanhaddad]

@justinhoward I really cannot imagine a use for that library beyond self-destructive inclinations.

[gandarez]

The only way to avoid error is to disable the flag noUnusedLocals which doesn't make sense at all

private readonly cacheStrategy;

constructor() {
    this.cacheStrategy = new ExpirationStrategy(new MemoryStorage());
}

@Cache(this.cacheStrategy, 300)
public async getApiKeyAsync(): Promise<string> {
...
}