Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

[eslint-plugin] Proposal: Add configuration helper #301

Closed
bartlangelaan opened this issue Feb 20, 2019 · 6 comments
Closed

[eslint-plugin] Proposal: Add configuration helper #301

bartlangelaan opened this issue Feb 20, 2019 · 6 comments
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@bartlangelaan
Copy link

To get this plugin and the TypeScript parser to work, currently it needs some basic configuration to be added to the ESLint configuration. This amount of configuration grows if it should only apply to TypeScript files, and it even increases more if you extend from a shared configuration (like airbnb's) that implements rules that aren't supported by the TypeScript ESLint parser.

With a configuration helper, we can enable everyone to automatically apply all Typescript-related stuff to your ESLint configuration. It would look something like this:

// .eslintrc.js

// You need to wrap your configuration with a function provided by the ESLint plugin.
module.exports = require('@typescript-eslint/eslint-plugin/config-helper')({
  
  // In there, include your normal ESLint configuration.
  extends: [
    'eslint-config-airbnb',
    'eslint-config-prettier',
    'eslint-config-prettier/react'
  ],
  plugins: ['prettier'],
  env: {
    browser: true,
  },
  rules: {
    'prettier/prettier': 'error',
    camelcase: ['error', { properties: 'never', ignoreDestructuring: false }],
    
    // You can also configure TypeScript rules here.
    '@typescript-eslint/restrict-plus-operands': 'error',
  }
});

Then, the config-helper function would apply all transformations needed to get the configuration working. So, the configuration that is actually exported looks like this:

{
  "extends": ["eslint-config-airbnb", "eslint-config-prettier", "eslint-config-prettier/react"],
  "plugins": ["prettier"],
  "env": { "browser": true, },
  "rules": {
    "prettier/prettier": "error",
    "camelcase": ["error", { "properties:": "never", "ignoreDestructuring": false }],
  },
  "overrides": [
    {
      "files": ["*.ts", "*.tsx"],
      "parser": "@typescript-eslint/parser",
      "plugins": ["@typescript-eslint"],
      "settings": {
        "import/resolver": { "node": {"extensions": [".js", ".ts", ".jsx", ".tsx", ".json"]}, "typescript": {}},
        "import/extensions": [".js", ".ts", ".jsx", ".tsx", ".json"]
      },
      "rules": {
        "camelcase": 0,
        "@typescript-eslint/camelcase": ["error", { "properties:": "never", "ignoreDestructuring": false }],
        "no-array-constructor": 0,
        "@typescript-eslint/no-array-constructor": ["error"],
        "import/no-unresolved": 0,
        "import/named": 0,
        "react/jsx-filename-extension": 0,
        "@typescript-eslint/restrict-plus-operands": "error"
      }
    }
  ]
}

So, what it basically does is:

  • Adds an override for .ts and .tsx files with the right parser and plugin
  • Checks which core rules are enabled (also in extends) that have a TypeScript-specific rule (like camelcase & no-array-constructor in this example), and disables the core rule and enables the TS-specific one with the original config.
  • Disables rules that are incompatible with or unnecessary for TypeScript (like import/no-unresolved and import/named in this example)
  • Moves all @typescript-eslint/* rule definitions to the override so they aren't enabled for non-TS files.

This way it's way easier to make ESLint compatible with TypeScript, as all needed config changes are handled for you.

Issues related to config, that would probably be helped by this helper: #36 #109 #118 #147 #154 #177 #265 #266 #267 #268

If others agree that this would be helpful, I'm willing to try to make a PR that enables this.

@bartlangelaan bartlangelaan added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Feb 20, 2019
@armano2
Copy link
Member

armano2 commented Feb 20, 2019

@bartlangelaan what do you think about making tool instead of wrapper?

we could do 2 things there:

  • migrate supported rules from tslint to eslint and generate config
  • check configuration for not supported rules and copy configuration with overrides
    • it will be nice if there will be way to statically generate desired file
    • and we can leave option to use it as wrapper

i don't like that this is going to be in plugin, maybe we should make separated package for this?

@bartlangelaan
Copy link
Author

I think a wrapper is the best option for most people, because if this project gets updated then the wrapper can automatically enable new rules if necessary. Users don't have to think about re-running the tool every time a new replacement rule is added to the plugin. Just updating the plugin would be enough.

I understand that it's a new thing and a new package could be good for that, but I think it's more user-friendly if the wrapper comes out of the box with the plugin. That's also because the wrapper is dependent on the right rules being available; it's easy to make the mistake of updating the wrapper without updating the rules, or the other way around.

I do think adding a tool that checks these things would be helpful for some, especially for power-users that want to configure every little thing themselves. It's a bit comparable to what eslint-config-prettier has: a little CLI tool that checks for rules that conflict with Prettier. If something like that would be added to this repo that would be awesome; but for 'normal' users I think it would be more friendly to also provide a wrapper. So that's what I would like to focus on in this proposal.

@bradzacher
Copy link
Member

Note that there's eslint/rfcs#9 which might help deal with this.


I don't think such a tool belongs directly in the plugin's package, though it could live in this monorepo.
We've gotten a lot of user feedback about mixed codebases, so it might be a worthwhile endeavour to improve ease of adoption.


  • Disables rules that are incompatible with or unnecessary for TypeScript

This is an unbounded set. There are a huge number of plugins which provide rules that might already be caught by typescript. I would recommend not providing a blacklist, or else we could possibly playing catchup with users reporting rules that are double reporting with TS's compiler.

Would mean we'd have to switch the list that gets disabled based on the plugins installed by the user. Note eslint will throw if you include rules that don't exist; so (depending on the list of rules from plugins) there's the chance that we'd have to detect plugin versions.

A bit of bike shedding, but something to think about.


Note that there is a lot of parts of this repo that will work on both JS and TS files.
The parser should work on both.
All of the override rules should work on both.
Rules inspecting TS specific constructs (interfaces, types) should work on both (because those nodes never show up).

Essentially there's one set of rules which will not work on JS files:
Rules requiring TS specific features (such as explicit-function-return-type) will not work on both.

@JoshuaKGoldberg
Copy link
Member

It'd be nice if the new package were written in a way that made it seamless to reuse its logic in a standalone CI helper, for teams that use JSON config formats...

@bradzacher
Copy link
Member

for teams that use JSON config formats...

From what I've been told - in future versions of ESLint JSON config files will be deprecated. I believe that they want to standardise on JS config files as they are easier to work with in general, and it simplifies ESLint's config resolution and parsing logic.

@JoshuaKGoldberg
Copy link
Member

Moving this to a discussion because we should hammer out what this configuration helper should look like. What is the API? How are you expected to call it?

@typescript-eslint typescript-eslint locked and limited conversation to collaborators Nov 25, 2022
@JoshuaKGoldberg JoshuaKGoldberg converted this issue into discussion #6090 Nov 25, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

4 participants