Configs: Switch all recommended values to errors?

[JoshuaKGoldberg]

Before You File a Proposal Please Confirm You Have Done The Following...

• I have searched for related issues and found none that match my proposal.
• I have read the FAQ and my problem is not listed.

Description

Per eslint/eslint#16512, a preference of at least three ESLint or plugin maintainers (myself included) is to use warnings only as a temporary measure:

Warnings were intended to be used temporarily. My belief is that using warnings long term results in people tuning them out as just console noise, so I don’t believe you are getting much benefit with long-term warnings.

Personally, I would advocate switching from warn to error in your strict config. As mentioned, warnings are easily ignored and thus not that useful other than for temporary/transitional usage. If someone opts-in to using your strict config, then presumably they're interested in actually enforcing stricter/more-opinionated rules with actual violations and not just warnings.

Proposal: let's switch meta.docs.recommended to just specify a recommended config family?

- recommended: 'error' | 'strict' | 'warn' | false;
+ recommended: 'recommended' | 'strict' | false;

Impacted Configurations

• recommended
• recommended-requiring-type-checking
• strict

Additional Info

No response

[bradzacher]

We've spoken about this before - errors should never ever be in the codebase, warnings should really not be, but it's not terrible if they are.

The recommended rules that are warning:

• no-unused-vars
• no-explicit-any
• no-non-null-assertion

I think no-explicit-any should probably be an error at this point of TS's maturity.
But the other two are more difficult.

Non-null assertions are a useful tool when used sparingly. We use them copiously in this codebase because TS API types aren't great, and neither are some of our types (eg .parent).

Unused variables are just a part of the development cycle, but they shouldn't exist in production code. The problem is that historically there were a number of tools that would hard-block the rebuild pipeline if they encountered ESLint errors. So by setting it to error you can end up getting in people's way and frustrating them!

I come from a perspective of lint at scale where it's impossible to enforce "lint 0" across the entire codebase, so leveraging the different severities is an important tool.
But I definitely understand that at most codebase's scale - warnings don't really have a good use beyond migration usecases.

So I'm in favour of this change.

[Josh-Cena]

I'm not in favor of no-non-null-assertion being an error—if anything I'd like to see this rule being removed from "recommended"... I've constantly seen people getting confused about the warning in legitimate usage places. This rule also interacts badly with poor CFA (a.k.a. microsoft/TypeScript#9998) and noUncheckedIndexAccess. What's the alternative? Change strings[k]! to strings[k] as string, which doesn't look any safer. To me, warnings mean "there's a fair chance it doesn't matter, feel free to suppress it", which is a much more friendly signal than an error.

[bradzacher]

One alternative to the assertion is just adding "unnecessary" checks to help TS refine the types.
People are really averse to adding unnecessary checks but ultimately defensively programming isn't the negative people make it out to be!

An anecdotal "case study" of sorts. Flow does not have a "non null assertion" operator. In fact there's no way to type-assert your way out of a nullish type like you can in TS because flow disallows unsound assertions.
The only way to handle it is via runtime checks.

So the entire Facebook codebase (the largest JS codebase in the world) is built with many "unnecessary" checks to help flow refine types.

To make it even worse - the graphql schemas have every field listed as nullable by default because technically any field could throw (which is silently logged and the value nulled out). In practice most of the fields won't throw though. So there's even more checks in the codebase than the average codebase.

Lots of "unnecessary" code that really doesn't harm anything or cause any measurable perf impact.