User libraries should be strict by default if written in TypeScript

[Ciantic]

My opinion is that:

• .ts files should be as strict as possible (you can do anything with as any trickery still)
• .js files should be free to do what ever you want.

Also I think there is no audience who loves working with non-strict TypeScript, it's mainly used by people who are migrating to TypeScript. Catering to "non-strict" TypeScript people is failure in waiting.

(Going in the reverse is more painful, because the libraries will then not type check, if we allow non-strict scripts to prevail.)

Point being that if you use TS you probably love the types and the strictness, if you don't you use JS anyway.

Opening this on suggestion by @kitsonk in #504

[kitsonk]

The TypeScript core team suggests that any new TypeScript project be enabled with strict that essentially it is a class of features that should have been in TypeScript from the start, but can't be, because of the large backwards compatibility situation. I believe DefinitelyTyped has gone to strict by default. When you scaffold a project using tsc the tsconfig.json defaults to strict.

Since TypeScript code in strict mode degrades well to sloppy mode, people can author for strict and still have it work in sloppy.

We support a custom tsconfig.json which would allow someone to run in sloppy mode, if they wish, as well as // @ts-ignore can be used to ignore specific errors. Everyone benefits from code being targeted at strict, so I am 👍 for enabling it.

[rsp]

I am strongly 👍 here, because I would love Deno to be the first runtime that is not suffering from The Billion Dollar Mistake, and without the strict flags we still get runtime errors like those, without any hint from the compiler that something can be wrong with the code:

Now is the only time when we can safely enable all strict flags by default. If we disable those flags now (which are not enabled by default by the TypeScript compiler itself only because of backwards compatibility but they are highly recommended by everyone) then after any significant amount of code for Deno is written and we need to maintain backwards compatibility, this decision will not be able to be changed later. Let's not make it one of the 10 things we regret about Deno few years from now.

Without flags like strictNullChecks we don't really have type safety at all. For example a function:

const f = (x: string) => x.toLowerCase(); // OK for compiler, unsafe on run time

doesn't guarantee that x is a string, but that it is either a string, a null or an undefined, which means that x.toLowerCase() can raise a runtime exception: TypeError: Cannot read property 'toLowerCase' of undefined - but to make it worse the compiler (or the editor) doesn't show any problem during the compilation which is very misleading.

If, on the other hand, TypeScript has all strict flags enabled, I still can have a function that takes null or undefined if I want but I have to be explicit about it, and I will get a compiler error when I try to invoke toLowerCase() method on something that is not guaranteed to have it:

const f = (x: string | null) => x.toLowerCase(); // COMPILE TIME ERROR
const f = (x: string | null) => x && x.toLowerCase(); // OK for compiler, safe on runtime

(instead of string|null or string|undefined people often use Maybe<string> or M<string> so it's not much more typing)

I believe it is a false sense of safety to use TypeScript without strict null checks, I even gave a talk titled: Call me irresponsible if I ever crash on null or undefined in JavaScript or TypeScript exactly about this very topic.

See also my comment in issue #504 here for some more details.

I am 👍 for all strict flags enabled in Deno for all TypeScript code.

[brandonkal]

FWIW I'm not a fan of forcing strict mode. It often means spending more time writing types with little gain. The compiler is rather good at inferring. StrictNullChecks should be on though.

Also if a library author doesn't use strict mode but you do, it won't work out.

[ry]

Thanks to @maxmellen for this work.

Let's see how annoying this is in practice... I am open to making strict mode non-default again if it turns out to be too verbose.

[Ciantic]

I would expect it to be annoying when migrating, because strict mode will find a lot of subtle bugs (and any assumptions that shouldn't be there).

With all new projects it should be easy since strict: true is the default (tsc --init) with TypeScript projects anyway.

[gewoonwoutje]

FWIW I was a bit surprised seeing compiler errors pointing to external URLs after updating from v0.33 to v0.34.

This could have a huge impact on type safety in third party libraries, I definitely see value in that.
I do think that a setting to disable the strict check for (specific?) external libraries would be nice to have.

[brandonkal]

That's just asking for trouble. If one of your deps is not strict, your whole project cannot be.

[kitsonk]

Per-module permissions/config/etc. are a rabbit hole I don't think we really want to go down. Non-strict code does no one any favours. If people can't get the upstream code to be strict, using tsconfig.json and setting "strict": false is what people should do in my opinion.

[Ciantic]

Whole purpose of this issues is to force TypeScript to be strict, it's the default. It will be painful with user code that is non-strict.

Problem is that there is now bunch of shoddily built TS deno code out there, it will be painful at the beginning, but strictness is default for TypeScript.

[gewoonwoutje]

True, and this should (hopefully) be a temporary problem anyway.

It is still a bit weird to explicitly turn of strictness in my entire project, because a library (I trust) has forgotten a type annotation for a function that may not even be exported to the module.

[Skillz4Killz]

+1 for strictness. If you use a non-strict lib, perhaps consider making a PR to improve that lib to make it strict.

[maxmellen]

I think it's worth mentioning that DefinitelyTyped's stance on this issue is that they consider that all the packages hosted in their repo should have noImplicitAny, noImplicitThis, and strictNullChecks enabled. (source)

Here's the default tsconfig.json file you get when adding a new package into their repository using the dts-gen utility:

{
    "compilerOptions": {
        "module": "commonjs",
        "lib": [
            "es6"
        ],
        "noImplicitAny": true,
        "noImplicitThis": true,
        "strictFunctionTypes": true,
        "strictNullChecks": true,
        "baseUrl": "../",
        "typeRoots": [
            "../"
        ],
        "types": [],
        "noEmit": true,
        "forceConsistentCasingInFileNames": true
    },
    "files": [
        "index.d.ts",
        "mypackage-tests.ts"
    ]
}

[brandonkal]

That doesn't really translate though as DefinitelyTyped is types for exported public functions. And it is for code that is not strict (JavaScript). Those rules don't mean all normal TypeScript code should be noImplicitAny especially for internal implementations.

[maxmellen]

@brandonkal You have a point there, although my comment was more of a reaction to @gewoonwoutje's comment.

What I wanted to stress is that it seems the TypeScript ecosystem is moving in the direction of users being able to expect that libraries expose strictly typed interfaces which can be used in strict as well as non-strict codebases rather than users having to deal with libraries possibly having non-strict types and figuring out how to use such libraries in strict codebases.

I'm also not aware of any TypeScript or Deno feature that lets a user specify which files should be considered part of a codebase's public API and which should be considered internal to that codebase, especially considering Deno does not require the use of any project configuration file.

I would argue that defaulting to strict makes new code more interoperable with the rest of the ecosystem by default.

[keroxp]

Recently I found many third-party library massively broken by that changes. I think --strict is too opinionated. It may be acceptable for stand-alone project but not for runtime, certainly.

The best advantage of TypeScript is that users can select suitable typing power for their project. Indeed we provide custom tsconfig.json for modules. But there are no way that "strict" module and "non-strict" module coexist. "strict" code can't be loaded by "non-strict" deno execution.

I'm OK to enable strict mode for deno's internal code (cli/std) as it is the stand-alone project and should be fully-maintained.

Again, "strict" is too opinionated for general purpose TypeScript runtime. Deno shouldn't force user to use specified compiler options.

IMO: At least --no-implicit-any should be disabled by default. It broke too many existing code bases.

[ry]

Being overly strict is painful for a scripting language. I would be using a scripting language to get things done quickly. If I wanted to be super-specific about what I was doing, I could just write it in Rust.

This is unrelated to strict mode - but just a general comment about overly pedantic rules: in the Deno code base we have a lint rule which requires writing the return types. So for many async functions we end up writing async function foo(...): Promise<void> ... The Promise<void> is just boilerplate - it make code unenjoyable to write.

Deno should be like a toy. Easy and simple to use.

I acknowledge that there are going to be situations where people want to change tsconfig - but we should strive to make that the minority case. Most people should be able to start script with one file, most projects should not have to ever think about configuration.

[kitsonk]

@ry if we reverse this, and I can understand the motivation, using strict should be fictionless as well... an easy step to upgrade your experience... If we introduced a --strict top level flag, so people can use it without having to load a config file, that would be a win.

Also, I would recommend we would continue to run --strict within the Deno code, including std.

[Skillz4Killz]

This is unrelated to strict mode - but just a general comment about overly pedantic rules: in the Deno code base we have a lint rule which requires writing the return types. So for many async functions we end up writing async function foo(...): Promise<void> ... The Promise<void> is just boilerplate - it make code unenjoyable to write.

@ry Why not disable that lint rule and allow TS to infer those? TS inference is really good, I have never ever needed to provide the return type of a function like that and I use TS strict mode always. It seems like that lint rule is causing more harm than helping.

Deno should be like a toy. Easy and simple to use.

100% agree. But every toy has certain standards, for example choking hazards for kids.

If I go to a toy store, I expect the toys there to meet a certain standard. It would be nice to have the ability to say that all Deno libs meet the TS strict standard.

[ry]

The fact that TypeScript is defaulting to strict mode, is a strong suggestion that we should as well. The incompatibilities that @keroxp is experiencing will go away as people update code to work with latest versions of deno - it can be chalked up to unfortunate growing pains.

We'll leave strict mode on by default for now - in particular @piscisaureus and @bartlomieju both are in favor of it. (And certainly we will always maintain very strict linting rules for Deno's internal code)

I worry about the user who has prototyped something quickly in JS, and now wants to start slowly adding types - but finds that when they rename their .js to .ts they suddenly have to update every function definition. It seems like a major benefit of typescript is lost if it's "all or nothing".

[brandonkal]

As I mentioned above, strict - noImplicitAny would be the best default especially for prototyping (you can let TypeScript infer types in most cases).
The latter can be found via an eslint rule or a flag to disallow implicit any.

[nayeemrmn]

I worry about the user who has prototyped something quickly in JS, and now wants to start slowly adding types

No need to worry, they have config. The temporary transition phase should of course be the one that requires config. That way the config is temporary.

[brandonkal]

Do you honestly expect someone to write a config file to prototype something? That's an unfair justification @nayeemrmn and just a bad user experience. Config is rarely ever temporary.

[nayeemrmn]

Do you honestly expect someone to write a config file to prototype something? That's an unfair justification @nayeemrmn and just a bad user experience. Config is rarely ever temporary.

@brandonkal Umm, no... the prototyper is using JS. We're talking about the person progressing from that, aren't we? :) Also I'm in favour of a flag.

Config is rarely ever temporary.

What does that mean here? I'm saying that strict is the one you're hardly going to go back from, non-strict might be temporary so it should be the specified one.

[hayd]

Config is rarely ever temporary.

This was @kitsonk 's suggestion of --allow-sloppy (alternative to a config), and I suspect calling it that will mean people will want its use to be temporary!

Potentially there could be a message on strictness errors suggesting this flag.

[Ciantic]

I agree with @nayeemrmn logic here.

When you are gradually adding types (converting between JS and TS) you can just add tsconfig.json with non-strict flags, that is already what TypeScript suggest you to do.

When you are done you can just remove tsconfig.json because conversion is done, and everything is strict!

[keroxp]

I agree with @ry and @brandonkal. And the real problem is that there are no way to apply configuration for each remote modules that depend on. There are incompatibility between strict code and "sloppy" code (for each other. Some valid code in strict may be invalid in non-strict mode). Custom tsconfig.json doesn't provide custom strictness to users. This means there are no choice to write "sloppy" code eventually.

IMO: If it were really the ideal and correct way of programming, its name won't be "strict".
IMO2: Disabling --no-implicitAny is better landing point of this issue currently. I'd been really tired for writing any or void on argument and return type.

[Ciantic]

Some valid code in strict may be invalid in non-strict mode

Really? What is that code you write in strict mode which is not usable in non-strict mode?

However the reverse is true: non-strict code is not usable in strict mode. This is the reason DT defaults to having more flags turned on.

If the deno defaults to non-strict mode the libraries start to gather non-strict code and you can never use those libraries with strict mode.

[keroxp]

@Ciantic

denodrivers/redis#50

Summary is here:

export type BulkResult = string | undefined;
export type RedisRawReply =
  | ["status", string]
  | ["integer", number]
  | ["bulk", BulkResult]
  | ["array", any[]]
  | ["error", Error];

function rawReply(): RedisRawReply {
  return ["bulk", "result"]
}

function getBulkResult(val: string | number | undefined): BulkResult {
  const [_, reply] = rawReply();
  if (typeof reply !== "string" && reply != null) {
    throw new Error();
  }
  return reply;
}

With strict:false tsconfig.json

deno run -c tsconfig.json strict.ts
Compile file:///Users/keroxp/src/deno-pg/strict.ts
error TS2322: Type 'string | number | any[] | Error' is not assignable to type 'string'.
  Type 'number' is not assignable to type 'string'.

► file:///Users/keroxp/src/deno-pg/strict.ts:18:3

18   return reply;
     ~~~~~~~~~~~~~

I don't what happened clearly. It seems that if statement was wrong but no error shown in strict mode.

[Ciantic]

@keroxp that has nothing to do with noImplicitAny, I pasted your code here:

https://www.typescriptlang.org/play/#code/KYDwDg9gTgLgBDAnmYcBCBXANgawErADO28AvHITFAJYB2A5nAD5wa0AmwAZncOwNwAoUJFgJkqAu2qE8AQwDuBMFkRxSguMzgBtAESU5MDIT0AaClTr0Aupu366MYPWBRzcWhgC2AIzd2Wiz6vtg4Hpi4BMRYMIEOenJQUHKIHnK0iDo28cF6blDQHgCiydA2QoJcbADGMNQQtHApSsAqiAAUAJQAXHBSMvKt7XAA3vZQwMZQTSFhHnqTMTB6dgC+glW19Y1wrjCR+EQkHQBuclh9lDQM2l5+btpsnDy0fL3oYdEkY-Y1jZRdAB9CyTdo2dTNRTKVTdIRaahcOAdJAoCBIsGqOAAQlI5AMVgYejgADISc02ljcZ5sFgur8tFoYAALQoKTzAdmlQpQOH2DZaSbTJqYxBCNZAA

And tune "noImplicitAny" on or off, it won't give errors in either mode. No errors in strict or otherwise.

None in this thread is actually anymore advocating for completly non-strict code btw, we are talking about noImplicitAny here mostly.

[kitsonk]

IMO: If it were really the ideal and correct way of programming, its name won't be "strict".

It is, according to the TypeScript core team, it is the correct way of programming. I've talked to them personally. Just like "use strict"; pragma in ECMAScript doesn't mean that strict mode isn't the ideal way of programming JavaScript, it was implemented to keep backwards compatibility and correct things that were actually regrets.

It is true that some strict code will break in sloppy mode. The specific example given undefined and null are not distinct types, so the comparison (reply != null) does not narrow the type to "everything not undefined). Because sloppy mode doesn't understand it, it re-widens based on the comparison outside the block, leaving you with everything possible. You can see when the assignment occurs, even though you specified undefined, that the return type of the function is string. It is exactly these type of things that were corrected in strict mode.

[vwkd]

I don't get why people allow TypeScript to prevent them from running code. You're not forced to listen to the TypeScript errors. You can always run your code using Deno's --no-check flag (which IMO should have been the default, but that's another topic).

TypeScript should be seen like a linter. They're helpful hints during development, but they don't prevent you from running the code at all at any time.

This is also why I'd vote for strict type checking, including no-implicit-any active by default. This doesn't "break" your project, it just prints squiggly lines that you can ignore if you want. Or you could actually use the squiggly lines to find the things that you forgot to type, which you should do, because this is the whole point of using TypeScript.

[kitsonk]

We have been run with strict on by default for an extended period of time and we have forgotten to close this issue.