[indent] behavior changed for multiline generic type parameters

[sqal]

Repro

module.exports = {
  root: true,
  extends: ['plugin:@typescript-eslint/recommended', 'airbnb'],
  parser: "@typescript-eslint/parser",
  parserOptions: {
    ecmaFeatures: {
      jsx: true
    },
  },
  plugins: ['@typescript-eslint'],
  rules: {
    indent: 'off',
    '@typescript-eslint/indent': ['error', 2],
  }
};

type ButtonAttrs = Omit<
  React.HTMLProps<HTMLButtonElement>,
  'size' | 'type'
>;

Expected Result
No indent error, should keep indentation of 2 spaces.

Actual Result

Expected indentation of 0 spaces but found 2. (eslint(@typescript-eslint/indent) [2,1]
Expected indentation of 0 spaces but found 2. (eslint(@typescript-eslint/indent) [3,1]

type ButtonAttrs = Omit<
React.HTMLProps<HTMLButtonElement>,
'size' | 'type'
>;

Additional Info

Versions

package	version
@typescript-eslint/eslint-plugin	1.7.0
@typescript-eslint/parser	1.7.0
TypeScript	3.3.4000
ESLint	5.16.0
node	11.11.0
npm	6.9.0

[mbret]

This issue is still occurring and prevent us to update @typescript-eslint/eslint-plugin. Any news about it ?

[bradzacher]

The short answer is - it's not getting worked on right now.

The long answer....

The current indent rule extension fudges things majorly by making typescript specific AST nodes "look like" standard JS nodes so that the base eslint logic can apply its logic to it.

This approach works okay for some things (like treating type declarations as if they were variable declarations, interfaces as if they were objects, etc).

However there are a number of things that have no standard JS nodes that even remotely look like it (such as generics). On top of that, the base rule logic does some things where it essentially zeros out the indentation if it encounters an unexpected node. This causes issues with things like type casts because they often sit inside member expressions.

So we need a better solution. We can't access the existing logic from within our rule, so we have to copy the code into this plugin. So far, I've forked the base indent rule code + tests from eslint core into this rule, and added types to them.

Unfortunately though, the rule is HUGE (rule's logic alone is ~1700 lines).

This means that it's a pretty huge task to add support for all of the typescript features because there's so much code to read through and understand.

I unfortunately don't have the time to tackle that right now. Happy for anyone to pick it up and give it a go.

[WoodyWoodsta]

Damn, this is rough.

[whatisaphone]

no standard JS nodes that even remotely look like it (such as generics)

What about changing the generics to function calls?

I ran a quick test. I started with this code:

export class WhatALongComponent extends React.Component<
WhatALongComponentProps,
WhatALongComponentState
> {}

const longVariablae = longFunction<
LongGeneric,
LongerGeneric,
LongestGeneric,
>();

Then I replaced both sets of generics <> with function-call parentheses ():

export class WhatALongComponent extends React.Component(
WhatALongComponentProps,
WhatALongComponentState
) {}

const longVariablae = longFunction(
LongGeneric,
LongerGeneric,
LongestGeneric,
)();

The I ran @typescript-eslint/indent's autofix:

export class WhatALongComponent extends React.Component(
  WhatALongComponentProps,
  WhatALongComponentState,
) {}

const longVariablae = longFunction(
  LongGeneric,
  LongerGeneric,
  LongestGeneric,
)();

Success!

[lostfields]

I got the same issue with indenting using generics, both of this is valid using indent of 4;

let list = [1, 2, 3]
    .map(value => {
        return String(value)
    })

let list = [1, 2, 3]
    .map<string>(value => {
    return String(value)
})

[juliendargelos]

Would it be possible to disable this behaviour using the ignoredNodes option ?

"ignoredNodes" accepts an array of selectors. If an AST node is matched by any of the selectors, the indentation of tokens which are direct children of that node will be ignored. This can be used as an escape hatch to relax the rule if you disagree with the indentation that it enforces for a particular syntactic pattern.

[andrey-semin]

any updates on this? This issue is getting really annoying for us

[bradzacher]

Happy to accept a PR

[julioolvr]

As a workaround (like @juliendargelos mentions) we'll ignore TSTypeParameterInstantiation using the ignoredNodes option and see how that goes. Mentioning in case it helps anyone in the meantime.

[beenotung]

@juliendargelos How do you use the option ignoredNodes? May you provide an example of the configuration?

I'm new to eslint and the IDE (JetBrain IDEA) cannot provide hints on .eslintrc.js, just treat it as a js file. Used to have hints like options suggestion and auto-complete on tslint.json

[juliendargelos]

@beenotung I guess you could use it this way (doc here):
@julioolvr can you confirm this is what you did ?

module.exports = {
  rules: {
    '@typescript-eslint/indent': ['error', 2, {
      ignoredNodes: ['TSTypeParameterInstantiation']
    }]
  }
}

[j3bb9z]

@juliendargelos I can confirm that I did it like this and it worked.

[beenotung]

I just tried above settings on indentation, it's still giving errors when I run eslint --fix, it seems easier if we disable indent check in eslint and use prettier for that.

[bradzacher]

@beenotung you definitely should disable all styling rules if you use prettier
I would use this to help you do so: https://github.com/prettier/eslint-config-prettier

There's no point having prettier and styling rules like indent on at the same time. Best bet is to do the same thing we do in this repo - run prettier as a CI check to help enforce formatting.

[beenotung]

@bradzacher I saw there are two conflicting setting recommended online.
Shall I put 'prettier' or 'plugin:prettier/recommended' as the last value in the 'extends' options?

[julioolvr]

@juliendargelos yes, we have pretty much exactly the same config

Fwiw, we're using prettier-eslint which runs prettier first and applies eslint's fixes on top of that.

[bradzacher]

The problem with prettier-eslint is that it's not well maintained prettier/prettier-eslint#275.
It doesn't support eslint6, and no doubt it also won't support the upcoming eslint7.

As for what you should use - IMO you should not use eslint-plugin-prettier: https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/FAQ.md#eslint-plugin-prettier
It causes a large perf impact in your lint runs, and provides little benefit over running prettier --list-different at commit time / on your CI.

Using eslint-config-prettier just turns off all rules in ESLint that conflict with prettier, so you should always use it if you're using prettier.

[bradzacher]

Merging into #1824