Changes to the `recommended` sets for 5.0.0

[bradzacher]

Similar to what I did for 3.0.0 (in #1423), I'm putting forward the new recommended set ahead of time.
I'm looking for feedback from the community before we go ahead and make the changes.

Hint: search for the text (new) to find the changes

Header Key

• New = new since version 3 - the last time we updated the recommended sets
• Ext = extension rule
• Dep = deprecated
  • ☑️ = deprecated in the next major
  • 🗑️ = to be removed from the plugin in the next version
• R = recommended
  • ⚠️ = recommended as warning
  • 🛑 = recommended as an error
  • 🗑️ = remove from recommended this version
• RWT = recommended with typechecking
  • ⚠️ = recommended as warning
  • 🛑 = recommended as an error
  • 🗑️ = remove from recommended this version

Rule	New	Ext	Dep	R	RWT	Comment
adjacent‑overload‑signatures				🛑
array‑type
await‑thenable					🛑
ban‑ts‑comment				🛑
ban‑tslint‑comment	☑️					There's still 3m weekly downloads for TSLint - too many users to make this recommended for now.
ban‑types				🛑		We should remove object from the default config, because enough people want to use it and disagree with the reasoning.
brace‑style		☑️
class‑literal‑property‑style
comma‑dangle	☑️	☑️				Not in ESLint's recommended.
comma‑spacing		☑️
consistent‑indexed‑object‑style	☑️					Stylistic.
consistent‑type‑assertions
consistent‑type‑definitions
consistent‑type‑imports	☑️					Stylistic.
default‑param‑last		☑️
dot‑notation		☑️
explicit‑function‑return‑type
explicit‑member‑accessibility
explicit‑module‑boundary‑types				🗑️ (new)		A lot of people are against this style and turn the rule off. It's entirely stylistic so we shouldn't recommend it.
func‑call‑spacing		☑️
indent		☑️
init‑declarations		☑️
keyword‑spacing		☑️
lines‑between‑class‑members		☑️
member‑delimiter‑style
member‑ordering
method‑signature‑style
naming‑convention
no‑array‑constructor		☑️		🛑
no‑base‑to‑string
no‑confusing‑non‑null‑assertion	☑️					Stylistic.
no‑confusing‑void‑expression	☑️					It's a good rule, but I think in general the errors it prevents will just be caught by the typechecker.
no‑dupe‑class‑members		☑️
no‑duplicate‑imports	☑️	☑️				Stylistic.
no‑dynamic‑delete
no‑empty‑function		☑️		🛑
no‑empty‑interface				🛑
no‑explicit‑any				⚠️
no‑extra‑non‑null‑assertion				🛑
no‑extra‑parens		☑️
no‑extra‑semi		☑️		🛑
no‑extraneous‑class
no‑floating‑promises					🛑
no‑for‑in‑array					🛑
no‑implicit‑any‑catch	☑️		☑️ (new)			TS 4.4 now has a compiler flag for this. So it's now a rule we don't need! Because 4.4 is still in RC, we'll just deprecate it, and remove it in the next major.
no‑implied‑eval		☑️			🛑
no‑inferrable‑types				🛑
no‑invalid‑this		☑️
no‑invalid‑void‑type
no‑loop‑func	☑️	☑️				Not in ESLint's recommended.
no‑loss‑of‑precision	☑️	☑️		🛑 (new)		ESLint added this to their recommended set.
no‑magic‑numbers		☑️
no‑misused‑new				🛑
no‑misused‑promises					🛑
no‑namespace				🛑
no‑non‑null‑asserted‑optional‑chain				🛑
no‑non‑null‑assertion				⚠️
no‑parameter‑properties
no‑redeclare	☑️	☑️				Recommended by ESLint, but we disable it in our eslint-recommended set.
no‑require‑imports
no‑shadow	☑️	☑️				Not in ESLint's recommended.
no‑this‑alias				🛑
no‑throw‑literal		☑️
no‑type‑alias
no‑unnecessary‑boolean‑literal‑compare
no‑unnecessary‑condition
no‑unnecessary‑qualifier
no‑unnecessary‑type‑arguments
no‑unnecessary‑type‑assertion					🛑
no‑unnecessary‑type‑constraint	☑️			🛑 (new)		Helps remove redundant code.
no‑unsafe‑argument	☑️				🛑 (new)	Same reasoning as the other no‑unsafe-* rules.
no‑unsafe‑assignment					🛑
no‑unsafe‑call					🛑
no‑unsafe‑member‑access					🛑
no‑unsafe‑return					🛑
no‑unused‑expressions		☑️
no‑unused‑vars‑experimental			🗑️ (new)			no‑unused‑vars now completely supports TS - so this rule is no longer required.
no‑unused‑vars		☑️		⚠️
no‑use‑before‑define		☑️
no‑useless‑constructor		☑️
no‑var‑requires				🛑
non‑nullable‑type‑assertion‑style
object‑curly‑spacing	☑️	☑️				Not in ESLint's recommended.
prefer‑as‑const				🛑
prefer‑enum‑initializers	☑️					Stylistic.
prefer‑for‑of
prefer‑function‑type
prefer‑includes
prefer‑literal‑enum‑member	☑️					Stylistic.
prefer‑namespace‑keyword				🛑
prefer‑nullish‑coalescing
prefer‑optional‑chain
prefer‑readonly‑parameter‑types
prefer‑readonly
prefer‑reduce‑type‑parameter
prefer‑regexp‑exec					🗑️ (new)	This is a stylistic rule and enough people have raised issues about its premise being wrong.
prefer‑return‑this‑type	☑️					Stylistic.
prefer‑string‑starts‑ends‑with
prefer‑ts‑expect‑error
promise‑function‑async
quotes		☑️
require‑array‑sort‑compare
require‑await		☑️
restrict‑plus‑operands					🛑
restrict‑template‑expressions					🛑
return‑await		☑️			🛑
semi		☑️
sort‑type‑union‑intersection‑members	☑️					Stylistic.
space‑before‑function‑paren		☑️
space‑infix‑ops	☑️	☑️				Not in ESLint's recommended.
strict‑boolean‑expressions
switch‑exhaustiveness‑check
triple‑slash‑reference				🛑
type‑annotation‑spacing
typedef
unbound‑method					🛑
unified‑signatures

[pgsandstrom]

I would prefer no‑inferrable‑types to be removed from the recommended list. I think it is debatable if it can be considered a best practice. Especially when I run into code such as this, where the rule just wants to make the code less uniform:

function test(
  a: number,
  b: number,
  c: number = 1,
  d: number = 1
) {
  //
}

Also consider issues such as #3145 and the rule can be quite annoying. The recommended list should be reserved for rules that are overwhelmingly more useful than annoying, and I dont think this rule makes the cut.

[joealden]

@bradzacher to clarify, this is for 5.0.0, not 4.0.0 (as we are already on version 4.x.x)? If so, then the title of this issue should probably be updated, and this issue should probably be added to the 5.0.0 milestone.

[bradzacher]

@joealden - ooops, thanks for that. We didn't change the sets in 4.0 so when I copy pasted my old issue from 3.0 I just incremented the version instead of thinking 🤦

@pgsandstrom perhaps we should instead add a new option to ignore function parameters and set it as the default - would that address your concerns with it being recommended?

[pgsandstrom]

I think that would improve the rule, but my personal opinion is still that the value it provides is a bit too opinionated and limited to be part of the recommended set.

[joealden]

To put in my 2 cents regarding no‑inferrable‑types, I've personally never found this rule annoying, and I personally think it should remain in the recommended rule set, as in my experience and opinion, it leads to more readable code 99% of the time.

---

Regarding a rule that is semi-related to no‑inferrable‑types, I almost always turn off explicit-module-boundary-types when using the recommended rule set, especially when I'm writing code that will never be published on NPM. My thinking regarding this rule is that it is useful for values that are accessible via a packages public API (as it prevents one from as easily making a breaking change without realizing it), but for modules that aren't exposed via a public API (either modules completely internal to a public package, or modules that exist within a codebase that isn't a library), I think this is overkill, as any module boundary type changes will be picked up the TypeScript compiler itself within the project.

I'm not saying that I think we should 100% remove this rule for the recommended preset, but in my experience, I turn it off most of the time, so I'm wondering what others think regarding this rule.

[JoshuaKGoldberg]

I almost always turn off explicit-module-boundary-types

+1 to this, I don't think I've ever enabled that rule.

[pgsandstrom]

I also always turn off explicit-module-boundary-types and it has been turned off at all major projects I've been working on for the last few years. I wouldn't mind it being removed from the recommended preset.

[bradzacher]

For me the point of the explicit-module-boundary-types rule is two fold:

First - readability. It means you don't have to rely on intellisense to figure out what a function is or does. This makes it easier to do reviews without an IDE. Code is read more times than it's written so IMO optimising for readability is paramount.

Second - it allows you to catch problems sooner.
By default TypeScript in the IDE does not check files you don't have open. So that means if you change the exports of file A, you don't find out that file B is broken until you manually run your build.

In some projects this is fine because you always have a webpack build in watch mode or something, but in others you have no build and must manually run tsc.
So the annotations force you to be explicit at the boundary and thus help catch some problems sooner - speeding the development process.

But I understand that some people find it cumbersome. I disagree with it personally, but I understand that some people like to heavily lean on the checker's inference.

This is one of the ways in which flow is better than TS (IMO). Flow has a mode called types-first which enables significant performance improvements by only analysing the types of a dependency's exports. For types-first you have to annotate the boundary, and it's a hard compiler error if you don't.
But alas - TS doesn't do any such optimisations.

---

But at this point it's pretty clear that this is a divisive topic and nobody agrees on it. Without some perf improvement from TS - it's not worth pushing the style - especially as there are so many people that are against it / turn it off.