docs(eslint-plugin): [consistent-type-definitions] add FAQs #10731
Conversation
|
Thanks for the PR, @JoshuaKGoldberg! 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. |
|
View your CI Pipeline Execution ↗ for commit b307a44.
☁️ Nx Cloud last updated this comment at |
| Almost never. | ||
| Most TypeScript projects do not -and should not- utilize types that exercise the performance differences between the two kinds of definitions. | ||
|
|
||
| If you are having problems with type checking performance, see the [TypeScript Wiki's Performance page](https://github.com/microsoft/TypeScript/wiki/Performance). |
There was a problem hiding this comment.
This is an incredibly useful page that I have never seen before 👌
packages/eslint-plugin/docs/rules/consistent-type-definitions.mdx
Outdated
Show resolved
Hide resolved
packages/eslint-plugin/docs/rules/consistent-type-definitions.mdx
Outdated
Show resolved
Hide resolved
kirkwaiblinger
added
the
awaiting response
JoshuaKGoldberg
removed
the
awaiting response
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10731 +/- ##
==========================================
+ Coverage 87.26% 87.30% +0.03%
==========================================
Files 450 450
Lines 15712 15758 +46
Branches 4600 4614 +14
==========================================
+ Hits 13711 13757 +46
Misses 1645 1645
Partials 356 356
Flags with carried forward coverage won't be shown. Click here to find out more. |
| For example: | ||
|
|
||
| - If your project is a dependency of another project that relies on a specific type definition style | ||
| - `Record<string, ...>` and `{ [i: string]: ... }` have subtle behavior differences |
There was a problem hiding this comment.
(coming from #10731 (comment)) Maybe this is just me not understanding, but I still don't understand how mentioning Records and index signatures fits into the flow of this document at all lol 🙃.
There is no prior mention of Record anywhere on the page, and so it's not clear to me at all as a reader how the Record-vs-index-signature really relates to types and interfaces. Especially since type Foo = Record<string, ...> and type Foo = { [i: string]: ... } are both types.
Even with the additional knowledge that Record is an alias for a mapped type, and index signatures may be specified on "object types" or on "interfaces", knowledge which I don't think we can assume on the user's part, I still genuinely don't follow.
Can this mention just be removed? Or can sufficient context be provided in order to make its intended meaning clear?
There was a problem hiding this comment.
If anything it feels like a caveat that was intended to be put in consistent-indexed-object-style, which currently does not have any such caveat mentioning that Record and index signatures are not always identical???
There was a problem hiding this comment.
(oops sorry missed this) yeah good point. Agreed this belongs more in the other rule, will remove for now. I think mentioning in the other rule's docs should be a followup. I'm having a hard time figuring out how to phrase the difference in the context of the dedicated rule...
kirkwaiblinger
added
the
awaiting response
github-actions
bot
removed
the
awaiting response
kirkwaiblinger
added
the
1 approval
PR Checklist
Overview
Adds FAQs with a comparison table explaining where they differ or are the same. I intentionally put the same number of ✅s for each side in the comparison table to try to convey that they're both fine to use, as long as a codebase remains consistent.
💖