Skip to content

Move opinionated rules from 'eslint-recommended' to 'recommended'? #592

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.

Sign up for GitHub

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

Jump to bottom
Closed
borekb opened this issue Jun 5, 2019 · 4 comments · Fixed by #595
Closed

Move opinionated rules from 'eslint-recommended' to 'recommended'? #592

borekb opened this issue Jun 5, 2019 · 4 comments · Fixed by #595
Labels
enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin recommended-rules Discussion about recommended rule sets

Comments

@borekb
Copy link
Contributor

borekb commented Jun 5, 2019

While looking at #591, I encountered something that I wanted to discuss separately.

Intuitively, I would think that a typical config could look like this:

{
  extends: [
    // start from this
    'eslint:recommended',

    // disable rules that are incompatible with TS or better handled by it
    '@typescript-eslint/eslint-recommended',

    // turn on TS-specific recommended rules
    '@typescript-eslint/recommended'
  ]

However, @typescript-eslint/eslint-recommended already contains some opinionated rules so adding it to our config actually yielded some new errors, which I wouldn't have expected.

I think I'd prefer if eslint-recommended was just a compatibility config while all the additional, opinionated and possibly stricter rules were in recommended. What do you think?
@borekb borekb added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jun 5, 2019
@borekb borekb mentioned this issue Jun 5, 2019
Merged
@bradzacher bradzacher added enhancement New feature or request recommended-rules Discussion about recommended rule sets and removed triage Waiting for team members to take a look labels Jun 5, 2019
@bradzacher
Copy link
Member

I'm okay with this!
But we'd have to wait for a major bump, as it's technically a breaking change.

@bradzacher bradzacher mentioned this issue Jun 5, 2019
Closed
14 tasks
@borekb borekb mentioned this issue Jun 6, 2019
Merged
@CvX
Copy link

CvX commented Jun 12, 2019

I like this proposal since, especially since currently adding "@typescript-eslint/eslint-recommended" to a config enables these rules using the overrides, which results in an unexpected behavior when you try to disable any of them, e.g.

{
  rules: {
    'prefer-const': 'off',
  },
}

…doesn't work. Instead, you have to also use an override:

{
  overrides: [
    {
      files: ['*.ts', '*.tsx'],
      rules: { 'prefer-const': 'off' },
    },
  ],
}

@bradzacher
Copy link
Member

Off topic, but out of curiosity @CvX - why don't you want to use prefer-const?

@CvX
Copy link

CvX commented Jun 12, 2019

Two reasons:

  1. I'm replacing tslint with eslint/typescript-eslint, so I'm getting a lot of new warnings/issues. What I like to do, is to disable most of them, and then incrementally deal with them, enabling and fixing them one by one.
  2. prefer-const in particular isn't that useful, since I care more about immutability rather than variable reassignment. The distinction between let and const often feels unnecessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin recommended-rules Discussion about recommended rule sets
Projects
None yet
Milestone
No milestone
3 participants
@CvX @borekb @bradzacher