-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[indent][meta issue] Problems with the indent rule #1824
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
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
* [eslint-config] add eslint-config package * Bump-up pnpify * [eslint-plugin] rename eslint-config to eslint-plugin * Setup ESLint * [eslint-plugin] use js instead of json to avoid yarn 2 resolution issue * [eslint-plugin] disabled @typescript-eslint's indent rule * bump eslint pnpify version * Fix format * [eslint-plugin] ignore max-len for comments * [eslint-plugin] add default option for typescript projects * [eslint-plugin] set quote-props rule as consistent-as-needed * enable eslint for workspace by default * cleanup gitattributes * [eslint-plugin] release minor ---- Issues: - Need to install plugins even eslint-plugin package provide it as dependencies. (yarnpkg/berry#8) - Cannot check indentations of TypeScript files properly. (typescript-eslint/typescript-eslint#1824)
when we first set up lining, I set an unconventional line-limit of 140 characters, and then accidentally wrote a commit message claiming it was 120. This commit changes the limit to actually be 120 characters, and changes code across many files to comply with this limit. I also had to disable eslint indentation checking for fields because it was enforcing an incorrect indentation style for wrapped lines. See the issue at typescript-eslint/typescript-eslint#1824
Ensures consistent indentation. [Documentation](https://typescript-eslint.io/rules/indent) !! This has [known issues](typescript-eslint/typescript-eslint#1824), revert to ESLint's version if necessary. This rule ensures: ```js function foo() { if (bar) { console.log('baz'); } else { console.log('qux'); } } switch(a){ case "a": break; case "b": break; } ``` Instead of allowing: ```js function foo() { if (bar) { console.log('baz'); } else { console.log('qux'); } } switch(a){ case "a": break; case "b": break; } ```
…ator (#222) ## Summary This resolves the following issue  ## Implementation Notes See typescript-eslint/typescript-eslint#1824
Hello, given that this page mentions the following:
Is fixing this rule still being worked on / actively supported? Thanks in advance. |
Given this issue has been open for 3 years with no community member pitching in (which is understandable!) - it's clear that this rule is not going to be fixed here. Following on from eslint/eslint#17522 With those two signals - I am going to lock this issue to prevent any further comments, and we shall not be accepting any PRs to fix the rule. I'll leave this open so it shows up in search, but consider it functionally closed. If you need further assistance with the rule - please consider posting on stackoverflow or similar. |
AirBnb only imports JavaScript rules and some fail for TypeScript files. This commit overrides those rules with TypeScript equivalents. Changes here can be mostly replaced when Vue natively support TypeScript for Airbnb (vuejs/eslint-config-airbnb#23). Enables @typescript-eslint/indent even though it's broken and it will not be fixed typescript-eslint/typescript-eslint#1824 until prettifier is used, because it is still useful. Change broken rules with TypeScript variants: - `no-useless-constructor` eslint/eslint#14118 typescript-eslint/typescript-eslint#873 - `no-shadow` eslint/eslint#13044 typescript-eslint/typescript-eslint#2483 typescript-eslint/typescript-eslint#325 typescript-eslint/typescript-eslint#2552 typescript-eslint/typescript-eslint#2484 typescript-eslint/typescript-eslint#2466
STOP! DO NOT FILE ANY NEW ISSUES ABOUT THE INDENT RULE
Your specific
indent
bug might not have an issue created. That does not matter. Don't file a new issue. If you raise an issue we will just close it and reference this issue.Please don't waste our time by doing that.
The rule is broken and telling us different ways in which it is broken doesn't change the fact that it is broken.
For the same reason - please don't comment on this issue with bug reports.
TL;DR - the
indent
rule is broken on TypeScript codebases. Due to bandwidth constraints it will remain broken indefinitely unless the community is able to step up to fix it.The Problem
Many of you that have used our
indent
extension rule have quickly run into problems with it.It incorrectly handles many cases that seem trivial, or it handles a block of code in one form, but breaks when you put in a new line. There are unfortunately a myriad of issues with it.
So when I first implemented it over a year ago (bradzacher/eslint-plugin-typescript#219), I took a very naive approach to solving the problem - I made TypeScript nodes look like JavaScript nodes, and called existing handlers in the base rule's implementation.
This worked well enough for simple cases, but was quickly discovered to be a terrible solution due to the complexity of TypeScript and the myriad ways in which its constructs can be nested and combined, and also in how users expect each combination to be indented by default.
So I began the process of forking the base rule codebase by copying it over and adding types (#439). I had grand plans to implement every single TypeScript node in this fork, so we could support it fully. This was another naive thought, as it quickly dawned on me how complex the rule's implementation is, and how difficult it is to handle every single variation of each node.
For example, take a simple union type:
Then there's the notion of configurability for this, which add more complexity.
The base rule has had ~5 years to grow via the community (though it was mostly a few dedicated contributors) - a community that actively uses the rule, and has motivation to use the rule because the problems cause issues for them.
A few years ago, I (like most of the maintainers) made the switch for all of my projects across to prettier. For me, this was spurred by my move to working at facebook, and learning to love the package and everything it does.
This means that I haven't used the rule since then, and I have no personal motivation as a volunteer maintainer to spend the many hours it will take to build this out, and as a maintainer there are many higher-priority issues (such as parsing perf) that need attention.
As such, the rule has languished, and many issues are open.
The Solution
The rule is in a broken state. Many users are working around it via ESLint disable comments, and configuring certain nodes as ignored; both of which are poor experiences.
We as maintainers have our hands full maintaining other core infra as well as triaging issues and reviewing PRs, so we don't have the bandwidth to take on this project.
We'd love it if some members of the community could help us out here by submitting some PRs to support TS nodes in the rule fork. Unfortunately, without some champions, the rule will not progress forward or be fixed.
Not to discourage anyone, but as a forewarning: there are some 2,100 lines of code already, and that just constitutes the typechecked fork of the base rule. The codebase contains some rather complex algorithms, and has a lot of moving parts. It will be no small undertaking to understand what it's doing, and how it's doing it.
The text was updated successfully, but these errors were encountered: