Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: How is @typescript-eslint/no-unused-vars different than TypeScript’s own errors? #4641

Closed
fregante opened this issue Mar 7, 2022 · 8 comments · Fixed by #7347
Closed
Labels
accepting prs Go ahead, send a pull request that resolves this issue documentation Documentation ("docs") that needs adding/updating good first issue Good for newcomers

Comments

@fregante
Copy link
Contributor

fregante commented Mar 7, 2022

Suggested Changes

Every unused variable is marked by both TypeScript and this rule. Why does it exist? It seems to be a duplicate. If it isn't a duplicate, can you explain why in the documentation?

'a' is assigned a value but never used. eslint[@typescript-eslint/no-unused-vars](https://typescript-eslint.io/rules/no-unused-vars)
'a' is declared but its value is never read. ts(6133)

It would also be great to include this explanation or a link to it in the documentation:

Affected URL(s)

https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/no-unused-vars.md

@fregante fregante added documentation Documentation ("docs") that needs adding/updating triage Waiting for maintainers to take a look labels Mar 7, 2022
@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue good first issue Good for newcomers and removed triage Waiting for maintainers to take a look labels Mar 7, 2022
@bradzacher
Copy link
Member

Every unused variable is marked by both TypeScript and this rule. Why does it exist?

Unfortunately TS has no concept of a warning. Everything is an error.

Typescript's compiler options create build time errors. If you use the no emit on error option then it means that having an unused local will prevent you from building.

In production builds this is great and correct! But in development where it is common to have half-finished code this can be a real workflow blocker.

OTOH eslint usually won't be setup to block build pipelines in dev. Which makes it much better for DevX.

Additionally there options for no-unused-vars - but TS can only be on, or off.

@fregante
Copy link
Contributor Author

fregante commented Mar 7, 2022

That makes sense, so you're saying that if I enable noUnusedLocals I should disable this rule?

@bradzacher
Copy link
Member

It depends on your setup and what your goal is.

noUnusedLocals/noUnusedParameters have an unconfigurable set of rules they follow that are entirely unconfigurable. For example: underscored names are always ignored and function params are ignored if following params are used.

However with no-unused-vars these things are configurable - you can choose a custom ignore pattern (it can just be ^_ if you like, or you can have NO ignore pattern - i.e. all variables must be used), and you can configure the param usability.

If you don't care about these things then you can probably just use noUnusedLocals and noUnusedParameters and turn off no-unused-vars, as long as you're okay with your build getting blocked in dev.

I generally advise that you use the lint rule instead of the TS option.
Note that TS will still hint about unused variables in VSCode by "greying out" unused variables without erroring thanks to the LSP integration:
image

@fregante
Copy link
Contributor Author

fregante commented Mar 7, 2022

Thanks for the detailed explanation!

I don’t think this has ever been a problem as unused variables can be very easily dropped, so I’ll stick to the strict TS config here.

OT: I wish there was such a rule for strictNullChecks though. That has been difficult to enable on existing repos.

@bradzacher
Copy link
Member

Unfortunately strictNullChecks actually changes the types that TS creates. TS will literally drop null | undefined from most union types without the flag!
This means a lint rule can't even help enforce anything or enable a gradual rollout because the types a rule looks at are just as borked.

@sukima

This comment was marked as off-topic.

@bradzacher

This comment was marked as off-topic.

@devinrhode2
Copy link

typescript seems to ignore variables starting with _, so I'm not sure what I would need to configure this rule for.

devinrhode2 added a commit to rye-com/rye-quickstart-tutorial that referenced this issue Nov 3, 2022
tsconfig settings:
  "noUnusedLocals": true,
  "noUnusedParameters": true,

typescript-eslint/typescript-eslint#4641
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue documentation Documentation ("docs") that needs adding/updating good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants