Skip to content
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

fix(eslint-plugin): [rules] Only enable rules on typescript files and include dot files #741

Conversation

chrisblossom
Copy link

@chrisblossom chrisblossom commented Jul 22, 2019

Typescript rules should only be enabled on typescript files. This is important for repositories with both normal JS and TS.

I ran into this problem when creating a universal eslint config for my projects. Typescript eslint rules were being enabled on normal javascript files causing a missing typescript dependency eslint error. I've manually imported them as a solution, added Typescript detection script but I think this should be fixed here.

This PR also enables overrides on .dot files for eslint < v6. See: eslint/eslint#11225 and eslint/eslint#11201

I've been meaning to put a PR in for sometime now, but #501 (comment) motivated me to put in now because this is might be considered a breaking change.

Looks to close #109
Similar: #615

@bradzacher
Copy link
Member

Thank you for your contribution!

I don't think this is the correct way to do things.
Doing so will make it very hard for people to adjust the recommended config.

Consider the following case:

{
  "extends": ["plugin:@typescript-eslint/recommended"],
  "rules": {
    // disable recommended rule
    "@typescript-eslint/ban-types": "off"
  }
}

The user's disabling of the ban-types rule will do nothing, because the override within the recommended config takes precedence, because it is more specific.

This means that instead a user has to know that the recommended config uses overrides, and use overrides themselves:

{
  "extends": ["plugin:@typescript-eslint/recommended"],
  "overrides": [
    {
      "files": ["*.ts", "*.tsx", ".*.ts", ".*.tsx"],
      "rules": {
        // disable recommended rule
        "@typescript-eslint/ban-types": "off"
      }
    }
  ]
}

This is obviously a sub-optimal experience, especially for those that have pure TS codebases.


I think that this is better fixed in user-land.
You can handle your mixed codebase within your config file, by only applying our recommended config to your \.ts(x)? files (as per #109 (comment), eslint6 now supports this)

{
  "overrides": [
    {
      "files": ["*.ts", "*.tsx", ".*.ts", ".*.tsx"],
      "extends": ["plugin:@typescript-eslint/recommended"],
    }
  ]
}

@bradzacher bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Jul 22, 2019
@chrisblossom
Copy link
Author

chrisblossom commented Jul 22, 2019

Thanks for the quick response! Also, I didn't see the previous discussion until after I submitted he PR.

I think it is unexpected behavior that the recommended rules run on .js, including changing the parser and sourceType for .js files.

Your concern about how an end user has to override has been fixed in v6.

I propose I modify this PR to use overrides for eslint >= v6 and the use previous behavior for older versions. I think that would solve everyone's concerns and work as expected moving forward.

@chrisblossom
Copy link
Author

chrisblossom commented Jul 29, 2019

@bradzacher What are your thoughts about what I proposed? If this isn't wanted, no problem at all, please close as I've solved it already for myself...just wanted to help others.

@bradzacher
Copy link
Member

bradzacher commented Jul 30, 2019

I'm not sure if it's right?
But TBH my experience with recommended configs is limited.
It'd be worth getting comments from some of the eslint peeps

cc @typescript-eslint/eslint-team

@phaux
Copy link
Contributor

phaux commented Aug 2, 2019

I don't see any reason why somebody would want to disable typescript rules for js files. They might be not as useful, but they're still beneficial.

@bradzacher
Copy link
Member

Hey there - sorry it's been so long.
I've had a think about this, and I think it's best if we don't do this, because it would affect many of our userbase.

As mentioned - the rules all work fine on JS code (there are only a v small list of exceptions, like explicit-function-return-type).
Additionally, there are many users that use these rules on things like markdown files, and vue files, which means they wouldn't be able to use the recommended sets.

None the less, thanks for the contribution!

@bradzacher bradzacher closed this Nov 12, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting response Issues waiting for a reply from the OP or another party
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for mixed JS and TS codebases - do not lint JS files
3 participants