Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Document the deprecation of no-unused-variable #1851

Closed
adidahiya opened this issue Dec 12, 2016 · 9 comments
Closed

Document the deprecation of no-unused-variable #1851

adidahiya opened this issue Dec 12, 2016 · 9 comments

Comments

@adidahiya
Copy link
Contributor

adidahiya commented Dec 12, 2016

https://palantir.github.io/tslint/rules/no-unused-variable/

We end up explaining how this rule is deprecated in favor of the compiler options in a lot of GH threads. We should use the deprecationMessage in the rule metadata:

deprecationMessage: "Use the tsc compiler options --noUnusedParameters and --noUnusedLocals instead.",


UPDATE (feb 27, 2017): the rule is no longer deprecated. see #2256

@schmuli
Copy link
Contributor

schmuli commented Dec 14, 2016

Just want to point out that there is one important limitation when using the compiler options, versus TSLint:

The compiler options are all-or-nothing: I cannot turn off the check for specific use cases.

For example, we have a folder of generated files with an import used by only part of the generated files, however the compiler will now mark all the generated files not using the import with an error. With TSLint, we could just add a comment to turn off checking for these files.

@nomaed
Copy link
Contributor

nomaed commented Jan 1, 2017

@schmuli As I mentioned in another place:

The problem with using the flag in tsc is that it causes an error during build, while the tslint rules is more configurable and can be used to produce warnings only.

So these are two use cases in favor of using the tslint rule.

@nknapp
Copy link

nknapp commented Jan 11, 2017

The third argument would be that during a development phase you are very likely to have unused-imports. I would like to run tslint in CI in order to detect problems in a pull-request before the branch is merged. The compiler option doesn't allow you to have an intermediate "broken" state when you try things.
Please keep the rule (and undeprecate it).

@adidahiya
Copy link
Contributor Author

Note that the compiler checks don't break your build or dev environment unless you set them up to do so either. tsc still transpiles JS that you can use in your project despite compilation errors.

@jsynowiec
Copy link

The compiler checks for unused params are unusable for me right now due to how the tsc treats unneeded parameters, please see #1861 (comment)

Where can I find reasoning behind depreciation of this rule, other than that TSC ca also do this?

@adidahiya
Copy link
Contributor Author

@jsynowiec from that thread:

The approved fix is to prefix uninteresting parameters with an underscore, see microsoft/TypeScript#9464 which breaks the variable-name TSLint rule.

There is a rule option exactly for this scenario called allow-leading-underscore. We can add it to the recommended config in the next release.

There is more discussion / explanation in #1481 and the linked issues. The rule implementation is buggy and the compiler does a better job with these checks, so the maintenance burden in the core ruleset doesn't feel worth it.

@mohsen1
Copy link
Contributor

mohsen1 commented Jan 25, 2017

Can variable-name know about unused params? Most of us don't want to allow leading underscore elsewhere

const _varName = 1; 
//    ~~~~~~~~       <-- this still needs to be a lint error
function foo(_: string, __: number, bar: boolean) {
  console.log('only interested in', bar);
}

@IllusionMH
Copy link
Contributor

I think that skipping just parameters with leading underscore is trivial fix(with new option) [works for case shown earlier].

However checking if it has leading underscore, but actually used (which should violate rule) - not so trivial and may be error prone.
I'll check it this weekend if nobody beats me.

@nchen63
Copy link
Contributor

nchen63 commented Mar 6, 2017

#1481 no longer being deprecated

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants