Repo: Enable TypeScript's "strict" mode

[JoshuaKGoldberg]

Coming over from https://github.com/eslint/rewrite/pull/117/files#r1742735782: TypeScript's compilerOptions.strict "enables a wide range of type checking behavior that results in stronger guarantees of program correctness. Turning this on is equivalent to enabling all of the strict mode family options... You can then turn off individual strict mode family checks as needed." Enabling strict is generally a TypeScript best practice. It's generally considered a must-have whenever possible: i.e. except when a pre-existing project is too large to easily migrate.

The two most impactful strict options are:

• noImplicitAny: tells TypeScript to produce type errors if it can't infer a parameter or variable from its default or initial values
  • Example: const log = value => console.log(value.toUpperCase()); log(123); produces a type error only with strictNullChecks
• strictNullChecks: makes null and undefined their own types, rather than implicitly allowed (the "billion dollar mistake")
  • Example: let value: string = null; console.log(value.toUpperCase()); produces a type error only with strictNullChecks
  • strictNullChecks in particular is necessary for a lot of typescript-eslint rules to run properly: search the intentionally obnoxiously named allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing in typescript-eslint.io

I understand there maybe some reluctance to enable strict on this repo. I think there was a discussion prior to #117 but I can't find it on demand. Are there strong reasons not to enable strict here?

[nzakas]

My main concern is the amount of effort required to meet strict mode out of the gate. At least in my (somewhat limited) experience, I've ended up running into issues where dependencies don't have type definitions and then I'm forced to create them (levn for example) or needing to reconcile types between packages we don't control (i.e., @types/eslint).

As someone who has a very limited number of hours I can devote to ESLint, spending those hours fighting with types doesn't seem like the best use of my time. (Or the team's time, where they may also be limited.)

It seems like we get around 80% of the benefit of type checking without strict mode.

That said, I'm not against strict mode in theory, I'm much more worried about it in practice. But I suppose if someone wants to come along and clean up my sloppy TypeScript behind me, I wouldn't mind that. 😄

I'm curious what others think.

[fasttime]

I'm basically in agreement with @nzakas. I'm not sure about the value of refactoring the code just to enable strict type checks. In the first place that would be an exercise in JSDoc syntax, although it's well possible that the strict checks would also reveal inconsistencies in the code.

But if we decide to write unit tests for our type definitions (and I think we should), then it would be good to enable the strict option for the type tests. That would ensure that the type definitions work well in projects that have strict type checks enabled.

[nzakas]

@mdjermanovic still waiting for your feedback here.

[mdjermanovic]

I agree to not enable strict mode. Seems that would require a lot of effort for the present and future code, and as said we already get a lot of the benefit of type checking without strict mode.

[bradzacher]

At least in my (somewhat limited) experience, I've ended up running into issues where dependencies don't have type definitions and then I'm forced to create them

If you don't have types that's okay - you can declare them locally if you want to strictly type everything (which is the general recommendation and best practice, of course).

But ultimately your local type declaration could simply be "add typings/levn.d.ts with content declare module 'levn';" and that's it! That will silence the TS error about missing module types and cause TS to treat the entire module as any.

---

I would very, very strongly urge you to at least turn on strictNullChecks.
Without it TS does not enforce any nullish safety eg

function test(arg: string | null) {
    // NO ERROR?!?!?
    arg.length
}

playground with strictNullChecks=false
playground with strictNullChecks=true

noImplicitAny is also a best practice just because it enforces all variables have a type that TS can infer/derive to avoid safety holes. However in a JS-first codebase it's understandable if you don't want this as it would require extra JSDoc annotations.

alwaysStrict is good if you have non-ESM files in your codebase as it will cause TS to always assume there's a "use strict"; directive - which adds extra safety. This is a no-brainer no matter your codebase.

strictBindCallApply is another no-brainer, IMO - as it enforces that .bind/call/apply are used correctly

strictFunctionTypes provides some nice extra safety for function types and is recommended. There isn't really a downside to turning it on, TBH.

strictPropertyInitialization enforces that a class property is either initialised in the constructor, inline in the property def, or is typed with | undefined to capture the non-initialistation case. The point is to prevent cases where you forget to init a variable causing the types to not match reality. The difficulty in a JS codebase is just ensuring all your properties are JSDoc typed correctly -- which you'll likely be doing anyways -- so it shouldn't be a problem to turn it on.

noImplicitThis will enforce your this's have an associated type. In a JS codebase you can probably not worry about this because it would require you to use JSDoc to annotate the this on function declarations / function expressions that are intended to be manually bound to class instances. If the plan is to only use class methods, class properties with function like values, and not use manually this bound functions then this should be easy to work with.

[nzakas]

@bradzacher thanks for the details on these. It seems like these options would be useful:

• strictNullChecks
• noImplicitAny
• strictBindCallApply
• strictFunctionTypes
• strictPropertyInitialization

alwaysStrict is good if you have non-ESM files in your codebase as it will cause TS to always assume there's a "use strict"; directive - which adds extra safety. This is a no-brainer no matter your codebase.

I'm not sure I understand this -- why would you assume "use strict" if it doesn't exist? You wouldn't want to apply strict-mode checks if the file itself isn't in strict mode? And doesn't the ESLint strict rule cover us here?

[bradzacher]

why would you assume "use strict" if it doesn't exist?

Sorry I missed this.
Because if you're transpiling the code with TS and the option is turned on then it will add the directive if it doesn't exist.
Thus ensuring that no matter what you do, your code is "always strict"!
But note that TS doesn't transpile JS -- so I believe the option would be useless for JS.