ni / javascript-styleguide

JavaScript and TypeScript Style Guide
MIT License
9 stars 9 forks source link

Problems with indent rule configuration #84

Closed jattasNI closed 2 years ago

jattasNI commented 2 years ago

In AzDO PR 260530 the SystemsManagement team reported problems with the default configuration of @typescript-eslint/indent. It turns out there are many known issues with how this rule behaves in TypeScript codebases. We should consider whether we're happy with our current configuration.

Some options:

  1. Leave the configuration as-is. Other repos have not reported problems so this may be good enough.
  2. Use the ESLint indent rule instead. This is the approach that SystemsManagement took, though I'm surprised it's any better. We'd want to do some testing and gather feedback before making this our default.
  3. Come up with a recommendation around .editorconfig, Prettier, or VS Code settings for indentation and disable the rule.
  4. Contribute a fix to the typescript-eslint issue linked above
  5. Turn off the rule entirely and let our codebases descend into anarchy ☹️

Here's one test case from the PR linked above: image

stefanc18 commented 2 years ago

Personally, from the cases I encountered so far, I think the eslint indent rule works great. There haven't been issues and is highly customizable, you can refer to its documentation for its options. In the Systems Management app, we applied a configuration of the indent rule similar to the one we previously had in tslint: "indent": ["error", 4, { "SwitchCase": 1 }], which applies 4 spaces indentation and also indents cases from switch statements. The second option shown in this issue's description uses this configuration.

JAYD3V commented 2 years ago

The problem with the eslint/indent rule is that it doesn't format TS nodes. But I have not had any problems using it for JavaScript nodes that exists inside of my TS code. It seems to be unaffected by TS syntax used in JS nodes, consequently; I came up, or came upon rather, the solution of formatting the indentation of TS nodes by hand. Its not that much work. I have found this to be the best solution, however, its far from perfect, despite having to format some stuff by hand, which, even if it is not a a large amount of more work, nevertheless, it is still more work, and any code formatted by hand is prone to human error.



There is one more issue:

I wanted to write a separate block for this issue because its about VS Code, and the issue surrounding VS Code that was mentioned in this post.

VS Code's vscode.languageFeatures formatter has historically had formatting issues w/ eslint. I really wouldn't worry about this too much. If a person writing TypeScript wants to use TypeScript, ESLint & a biased formatter inside of the Visual Studio Code dev environment, they can, and they can do so with out having problems w/ this rule. Prettier would be required as a dev-dependency in the person's projects, and the ESLint-Prettier extension would need to be installed, but that can be done in two clicks inside of VS Code. but that solves the issue dealing with VSC & VSC's formatter. Just FYI, this is a common work around for times when ESLint has a problem with a typescript formatter, be it JS or TS.


To Conclude

IMHO, this rule can be used to format TypeScript. The situation is not perfect, but it is very manageable.

jattasNI commented 2 years ago

Our current thinking is to deprioritize this issue because we haven't seen many reports of it being a problem. It seems to only be a problem on certain specific syntaxes.

I'll close the issue for now but we'll continue to monitor conversation on it if people would like to advocate for a different decision.