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

Additions to eslint:recommended #477

Closed
5 tasks
ThomasdenH opened this issue Apr 26, 2019 · 3 comments · Fixed by #488
Closed
5 tasks

Additions to eslint:recommended #477

ThomasdenH opened this issue Apr 26, 2019 · 3 comments · Fixed by #488
Labels
enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin recommended-rules Discussion about recommended rule sets

Comments

@ThomasdenH
Copy link
Contributor

There are some rules in eslint:recommended of which the configuration could be overridden here. I think there are two main categories here:

  1. Typescript enables more modern Javascript, so rules recommending this can be enabled.
  2. Typescript already has the equivalent of a rule, so the rule should be disabled.
  • no-undef: Typescript already checks this, so this rule can be disabled.
  • no-var: Typescript always allows let and const which have considerable benefits, so they can be recommended by default.
  • prefer-const: If possible, const is more restrictive and thus easier to reason about.
  • prefer-rest-params: Since the spread operator is available in Typescript, recommend its usage instead.
  • no-prototype-builtins: This is perhaps more personal taste than the others as it doesn't fall in either category, but whereas Typescript normally creates type-safe code, Object.create(null).hasOwnProtoperty(x) can still cause TypeErrors. This can be circumvented by recommending Object.prototype.hasOwnProperty.call instead.
@ThomasdenH ThomasdenH added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Apr 26, 2019
@ThomasdenH
Copy link
Contributor Author

@bradzacher bradzacher added enhancement New feature or request recommended-rules Discussion about recommended rule sets and removed triage Waiting for maintainers to take a look labels Apr 26, 2019
@ThomasdenH
Copy link
Contributor Author

There are some considerations to be made here regarding the implementation. One possibility would be to simply require the user to use eslint:recommended, either themselves or extend it from this repository's config file. But to better suit more people, it might be preferable to create a separate config file with the goal of updating the recommended set of default rules.

Any thoughts on this?

@bradzacher
Copy link
Member

bradzacher commented Apr 29, 2019

I'm not sure if it's a good idea to be disabling rules that don't have their own implementation in this plugin, if only because we know a large number of users have mixed codebases.
Eslint currently doesn't support nesting an extends within an overrides eslint/eslint#8813, which means we could be disabling rules the user actually wants to be using for their non-ts code.

Would probably be good as a separate config for these things to make it easy for people to opt into it.

I'm unsure how much weight we should be putting behind mixed codebases with the recommended config.
Maybe we should have recommended as the default (which assumes single language codebase, and does stuff like you've mentioned) and add a "recommended-mixed" config which doesn't do this stuff...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

Successfully merging a pull request may close this issue.

2 participants