-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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: [no-unnecessary-type-parameters] add FAQ section #9975
docs: [no-unnecessary-type-parameters] add FAQ section #9975
Conversation
Thanks for the PR, @kirkwaiblinger! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 88aa43f. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
0563798
to
0b39079
Compare
0b39079
to
88115e8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9975 +/- ##
==========================================
+ Coverage 88.69% 88.71% +0.02%
==========================================
Files 425 426 +1
Lines 14810 14829 +19
Branches 4308 4313 +5
==========================================
+ Hits 13135 13155 +20
Misses 1531 1531
+ Partials 144 143 -1
Flags with carried forward coverage won't be shown. Click here to find out more. |
You might be surprised to that the rule reports on a function like this: | ||
|
||
```ts | ||
function f<T extends string>(string1: T): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love if someone has a better example for this. I didn't just copy the example from #9828 since it didn't seem very organic, but I'm not sure this is any better 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I normally just go with log
or identity
. 🤷
function log<T extends string>(string1: T): void {
const string2: T = string1;
console.log(string2);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM so far! Just waiting on the todo note.
I was thinking to defer that to a followup once the linked conversation is fully resolved. Thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Let's do it!
PR Checklist
Overview
Add FAQ section to rule's doc page.
Tried to mention all/most of the issues linked to #9680 in some capacity.