[FEAT] Recommended config #261
Conversation
- turn off conflicting base rules - add additional config
|
@aladdin-add - I have updated the readme |
|
@bradzacher looks like there are errors in tests for |
# Conflicts: # lib/rules/no-type-alias.js # lib/util.js
# Conflicts: # README.md # lib/configs/recommended.json # lib/rules/no-unused-vars.js
|
It does not work for me. ESLint complains that "warning" should be "warn" instead. Bonus question: how to make it work with prettier? |
|
it was fixed in master, but not released yet. @bradzacher plan to release a patch version? |
|
I'm back from holidays so I'll look to cleaning things up and releasing |
| "typescript/explicit-function-return-type": "warning", | ||
| "typescript/explicit-member-accessibility": "error", | ||
| "indent": "off", | ||
| "typescript/indent": "error", |
There was a problem hiding this comment.
recommended was designed to only contain possible errors and best practices rules, the typescript/indent as a stylistic rule, I'm not sure if it could be included in the recommended.
There was a problem hiding this comment.
I agree. Although it seems like all remommended rulesets out there include stylistic rules as well, probably for the folks who don't use prettier. I think the best solution would be to leave these rules in recommended, and add plugin:typescript/prettier to shut them off. Thoughts?
I also noticed other differences compared to tslint:recommended, for example complaining about using IFoo as interface name instead of Foo. Correct me if I am wrong, but IFoo seems like the more popular convention, although on typescript docs site I don't see them used.
There was a problem hiding this comment.
There’s an eslint-config-prettier, which I’m going to add a typescript preset to (prettier/eslint-config-prettier#68), which will disable all stylistic rules. People could then extend ["plugin:typescript/recommended", "prettier/typescript"].
There was a problem hiding this comment.
we should not enable stylistic rules as recommended, they are opinionated, some ppl prefer tabs over spaces xd
There was a problem hiding this comment.
As a general rule, we turn on rules based on what is considered best practice - which we've been deriving from the TS team (their docs and the playground).
The goal of eslint is to help developers enforce consistency on their codebase.
@arvigeus - Using IFoo is an antipattern; a naming convention derived from languages like C#/Java where interfaces are pretty much a tangible thing at runtime; so you don't want people to confuse them with classes.
In TS; interfaces are compile-time only. Also (when it comes to object typing) there is very little difference between interface Foo and type Foo, which begs the question of why you'd choose to name it interface IFoo, but name types type Foo.
This is the style the typescript docs have been using for a long while, so we recommend people follow that.
As for indent - typescript docs use 4 spaces, as does the typescript playground, hence we recommend that too. Additionally, I'm yet to see a TS codebase that doesn't enforce some form of indentation standard.
Whilst everyone is different, turning on the rule by default serves two purposes:
- it alerts the user that there is in fact an indentation rule, so they can reconfigure it if they want tabs /
nspaces. - it lets us ensure that they have disabled the base eslint indentation rule for them (in case it was turned on via something like
eslint-config-airbnb).
There was a problem hiding this comment.
in particular for the second point. There are three possible cases:
- Don't disable
indent, don't turn ontypescript/indent.- If they apply the airbnb config, then ours, and we don't disable
indent, then they'll get a number of incorrect indentation errors due to the incorrect support of TS nodes. This could possibly lead to users opening issues because of the incorrect handling from the base rule.
- If they apply the airbnb config, then ours, and we don't disable
- Disable
indent, don't turn ontypescript/indent.- This is terrible because it means the user will lose their indentation errors, and they mightn't even know we disabled them.
- Disable
indent, turn ontypescript/indent.- Stops users using the broken rule, and starts flagging errors. User knows the rule is misconfigured for their code, and adjusts accordingly.
Looking at some stats - of the top 292 starred TS repos; 45% use 4 spaces, 36% use 2 spaces, and 16% use tabs (0.68% uses 3 spaces - wtf).
So with this setting we're likely targeting the most common use case.
There was a problem hiding this comment.
- Disable indent, turn on typescript/indent.
it has a small fault: we could disable indent for ts files, but not js files. (given some repos mixed using js and ts)
another way might be considered is Don't disable indent, don't turn on typescript/indent, but give users a guid to disable indent explicitly:
{
"extends": ["plugin:typescript/recommended"]
"rules": {indent: "off"}
}There was a problem hiding this comment.
I don't have any data, but my gut would say that the majority use case is TS only. For those that have a mixed codebase, they're probably advanced enough users that they can apply overrides themselves (not that they need to, typescript/indent works fine with plain JS code).
Turning off the base rule is listed as part of the config in the docs for our indent rule..
I don't like the idea of only disabling indent, whether it be on everything, or just TS(X) files, because unless a user interrogates the recommended config, they won't know we disabled it, and could then fall into using inconsistent indent when they thought their linter was working.
There was a problem hiding this comment.
what's about vue files? more and more ppl is moving to typescript :)
vue + ts + eslint
if you limit it to ts only it's going to stop working for them
Fixes #144
Requires
#259,#260.recommendedprop to be"error" | "warning" | falsetrue. otherwise it's just a property that isn't used officially by eslint. It's truthy soeslint-docsstill work."error"/"warning"for more configurability.