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

[monorepo] Remove "noUnusedLocals" and "noUnusedParameters" from "tsconfig.base.json" #4871

Closed
Zamiell opened this issue Apr 26, 2022 · 0 comments · Fixed by #4882
Closed
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working good first issue Good for newcomers repo maintenance things to do with maintenance of the repo, and not with code/docs

Comments

@Zamiell
Copy link
Contributor

Zamiell commented Apr 26, 2022

Recently, I have been working on some PRs for typescript-eslint, and I have noticed a bug in the setup of this repository.

In the tsconfig.base.json file, the "noUnusedLocals" and the "noUnusedParameters" compiler flags are specified. Normally, turning these on these flags provides extra safety for a TypeScript project. However, if the repository also uses the "@typescript-eslint/no-unused-vars" rule, then these compiler warnings become (mostly) superfluous.

For example, this unused variable triggers a buggy rule overlap:

image

The solution for this is to either turn the compiler flags off, or to turn the ESLint rule off.

This conflict is discussed in more detail in issue #4641 by Brad. Brad says that in general, he recommends that you use the ESLint rule over the compiler flags. This is what I have been doing in my own projects as well. I like it because ESLint has the feature where you can quickly ignore the warning by prefixing the variable with an underscore. (The TypeScript compiler does not understand this, and will continue to warn you about the variable even after putting an underscore on it.)

I think Brad's advice should also be applied to this monorepo as well, thus this issue.

Finally, a maintainer should remove the package: eslint-plugin tag; I had to choose this arbitrarily since there did not seem to be an issue template for issues relating to the monorepo itself.

@Zamiell Zamiell added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Apr 26, 2022
@JoshuaKGoldberg JoshuaKGoldberg added repo maintenance things to do with maintenance of the repo, and not with code/docs accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working good first issue Good for newcomers and removed package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Apr 29, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2022
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 bug Something isn't working good first issue Good for newcomers repo maintenance things to do with maintenance of the repo, and not with code/docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants