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

feat: use typescript-eslint's recommended-requiring-type-checking #1035

Closed
JoshuaKGoldberg opened this issue Dec 31, 2022 · 3 comments · Fixed by #1036
Closed

feat: use typescript-eslint's recommended-requiring-type-checking #1035

JoshuaKGoldberg opened this issue Dec 31, 2022 · 3 comments · Fixed by #1036
Labels
🌟 enhancement New feature or request

Comments

@JoshuaKGoldberg
Copy link
Contributor

Is your feature request related to a problem? Please describe.

https://github.com/t3-oss/create-t3-app/blob/88ce6e50289281e009f1eea2223dbef4cb86df61/cli/template/base/.eslintrc.json right now extends from plugin:@typescript-eslint/recommended. But per typescript-eslint.io's docs on typed linting there's actually a second recommended ruleset that can be enabled: plugin:@typescript-eslint/recommended-requiring-type-checking (RRTC).

The RRTC config isn't enabled by default because typed linting comes with some performance drawbacks. But it includes many of the particularly powerful rules in typescript-eslint. See https://typescript-eslint.io/rules for a full list.

Describe the solution you'd like to see

trpc/trpc#3482 is a pretty good reference point:

  • Add "plugin:@typescript-eslint/recommended-requiring-type-checking" to the extends in the ESLint config file
  • Have tsconfig.eslint.json extend from tsconfig.json, to pull in relevant compiler options
  • Disable rules that are either undesirable or have way too many existing non-auto-fixable complaints
  • Fix up any remaining complaints

Describe alternate solutions

Going without the fancier, stronger rules, I suppose? 🤔

Additional information

See #317 for typescript-eslint's introduction.

@JoshuaKGoldberg JoshuaKGoldberg added the 🌟 enhancement New feature or request label Dec 31, 2022
@c-ehrlich
Copy link
Member

Thanks for opening the issue. This sounds great overall. There are a few strict settings that don't make sense to me in certain projects (for example banning non-null assertions, when the only place you were ever going to use them is places where you'll still use them anyway but now with two lines of comments above to allow it) but overall I'm strongly in favor of it, and I could probably have my mind changed about any of the ones that I don't love.

If you're up for it, could you make a PR of an implementation with you proposed rule set so we can see both how many changes it requires for a stock app, and what it looks like with a "dirty" existing project?

By the way, would this also make sense for the CLI?

@JoshuaKGoldberg
Copy link
Contributor Author

JoshuaKGoldberg commented Dec 31, 2022

There are a few strict settings that don't make sense to me in certain projects (for example banning non-null assertions, when the only place you were ever going to use them is places where you'll still use them anyway but now with two lines of comments above to allow it)

Yeah 😞 no-non-null-assertion is tentatively likely to be moved to the strict ruleset. Conversation here that I think agrees with where you're coming from 😄 - typescript-eslint/typescript-eslint#6014 (comment).

If you have any input on the recommended rulesets there, I'd love to hear it. We're going to publish that suggestion list out soon for community feedback.

PR of an implementation

Is #1036 what you're looking for? By "dirty" existing project do you have a particular one in mind? I'm not sure I follow.

would this also make sense for the CLI?

I think so - if you mean https://github.com/t3-oss/create-t3-app/blob/d38c78624d502e648eeb80bc595bbc9c3c686f1e/cli/template/base/.eslintrc.json?

@c-ehrlich
Copy link
Member

Is #1036 what you're looking for? By "dirty" existing project do you have a particular one in mind? I'm not sure I follow.

Yes, that's great! By a "dirty" project I just meant pasting these rules into an existing codebase where they weren't being used, and seeing how much if any existing code is flagged by the rules. I gave this a try, and it seems good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants