-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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: update with TypeScript info #17423
Conversation
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks for the updates. I don't think linking to the JSON file is helpful and for all we know that location could change.
Is there some documentation you could link to instead?
Hi nzakas, I don't know if there is generic documentation for TypeScript error types. I googlged it and found this StackOverflow page that just links to the JSON file. For now, I'll just edit the PR and remove the link. Can you respond to the topics that I raised in the "Other Discussion" section? |
I think the typescript-eslint repository would be better place to maintain this list and maybe even provide a configuration that disables these rules. There's already a configuration that disables |
@mdjermanovic I think that they have already decided against this; see this issue. |
Fair, but after reading typescript-eslint/typescript-eslint#6449 (comment), I'd agree with their suggestion that this should be a separate project as I'm not sure we could afford following versions of TypeScript and testing whether the rules we mark as safe to disable when using TypeScript are indeed completely covered by TypeScript. |
@mdjermanovic I appreciate the concern about wanting to have 100% test coverage, but I don't think this has to be all that complicated. It is technically possible that some future TypeScript version turns out to make the type checker less strict, which would subsequently make my PR here outdated... but that seems pretty darn unlikely! Hopefully we can agree on that. I agree that ESLint should not have to bother itself with specifying the exact TypeScript versions that a check applies to. It's enough to just say that TypeScript covers it, and we can expect the reader do more research themselves if they really need to. I think the Pareto principle applies here: even if the passage on TypeScript in an ESLint rule page is not super in-depth, it is still a great starting point to alert ESLint users of a potential problem. Now, on the topic of putting the documentation somewhere else than the ESLint rule pages: I think that sucks for end-users. Consider the case of someone reading the ESLint rule docs, going over every rule to decide which ones they should turn on in their codebase. Naturally, the kind of person who is scouting out for good ESLint rules in order to keep their codebase error-free is exactly the kind of person who is already going to be using TypeScript. The state of affairs right now is that some of the ESLint rules mention TypeScript incompatibility properly. And others do not. This makes the "scouting" experience for this user a real pain. And even in a hypothetical where the "scouting" user has a solid 3rd party resource that contains core ESLint rule incompatibility issues with TypeScript (e.g. Counterargument: Doesn't this create more of a documentation maintenance burden on the ESLint core team? Will ESLint core team be blamed if the documentation isn't perfect? Well, I would argue no. There are only a few core rules that are known to be currently covered by TypeScript, and getting those documented is a pretty small undertaking. Beyond that, I would argue that the ESLint core team is not responsible for keeping a master list of TypeScript incompatibility; rather, the existing incompatibility documentation is just a community-based best-effort. Again, I feel that the Pareto principle strongly applies here: any small mention of TypeScript incompatibility on the official ESLint rule pages will go a very long way. |
I'd say let's open a separate issue to discuss whether or not we want to track |
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[X] Documentation update
What changes did you make? (Give an overview)
I added TypeScript information to the docs.
Is there anything you'd like reviewers to focus on?
No.
Motivation
The
no-dupe-class-members
rule helpfully documents that if you are using TypeScript, then you don't have to enable the rule. This is fantastic! However, there are many other ESLint rules that are also pointless if you are using TypeScript, but they are not documented. So this is the first step in solving that problem.Other Discussion
I want to document all rules that are covered by the TypeScript compiler, and this PR is the first step in doing so.
In my next PR I would like to reduce the text duplication that I am introducing with this PR by using some kind of auto-filled template. I envision something like:
Which would output the following text during the actual website build:
I don't know if the docs already contains anything like this, so I would like a contributor to work with me to find the cleanest way to accomplish this.
Furthermore, I also want to include a TypeScript icon next to these kinds of texts to make it easier to see at a glance that the rule is covered already by TypeScript, which is another idea for a future PR.