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

Remove no-unused-variable rule in v5.0 #1617

Closed
adidahiya opened this issue Oct 9, 2016 · 4 comments
Closed

Remove no-unused-variable rule in v5.0 #1617

adidahiya opened this issue Oct 9, 2016 · 4 comments

Comments

@adidahiya
Copy link
Contributor

Follow-up from #1481.

@mprobst
Copy link
Contributor

mprobst commented Feb 3, 2017

@adidahiya we're using no-unused-variable in Angular and at Google. We're aware of the compiler errors, but we actually prefer the linter warning quite a bit, consistent with feedback in #1481. We universally run with --noEmitOnError (or an equivalent thereof), so not having compilation fail on unused variables, but rather having it has a lint error, is quite important to us.

Removing this would be quite annoying for our workflow - we'd probably actually backport the rule into our own code base. Given that, maybe it'd make sense to document on no-unused-variable that you can get a similar effect with the compiler options, but un-deprecate it and keep it around? The linter warning enables a very different workflow.

@alexeagle
Copy link
Contributor

+1, because TypeScript doesn't have warnings, we have to use tslint for the checks that don't block the developer. IMO only issues that guarantee the program executes incorrectly should block developers (because you always benefit from catching these before you debug your tests or manually test)

I propose that we keep these rules in tslint (and we need to suppress the warning they print, since we will still use them), at least until TypeScript has a mechanism for warnings (which I also have a lot of ideas about :)

@ghost
Copy link

ghost commented Feb 27, 2017

I agree with that! In react I sometimes want to comment a part of the JXS to isolate a problem and a lot of vars become unused. I just need a warning in that case.

mprobst added a commit to mprobst/tslint that referenced this issue Feb 27, 2017
As discussed in feedback on  palantir#1481 and palantir#1617, for many users `--noUnusedParameters` and `--noUnusedLocals` do not work as a replacement for `noUnusedVariable`. This change de-deprecates the rule and documents the alternative compiler flags (and why they might not work for users) in the description.

Fixes palantir#1617.
@adidahiya
Copy link
Contributor Author

Cool, looks like there's a lot of interest in keeping the rule around in the thread in #1481. Going to mark this as "won't fix" to signal that we're not deprecating the rule. It's still going to be opt-in though, and not enabled in the default configs.

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

3 participants