NonNullable<null> is null without strict null checks

[dummdidumm]

Bug Report

NonNullable<null> was never in all cases previously, since 4.8 it's null when strictNullChecks: false. From reading the PR that changed the implementation of NonNullable I can't really tell if this is now expected or an oversight. If it's expected, maybe the blog post could be updated because I think this is a significant difference in behavior.

🔎 Search Terms

NonNullable never null

🕗 Version & Regression Information

• This changed between versions 4.7 and 4.8

⏯ Playground Link

Playground link with relevant code (same behavior for 4.7, but the implementation of NonNullable was different there)

💻 Code

type NonNullableNew<T> = T & {};
type NonNullableOld<T> = T extends null | undefined ? never : T;

type IsNullWithoutStrictNullChecks = NonNullableNew<null>;
type IsAlwaysNever = NonNullableOld<null>;

🙁 Actual behavior

NonNullable<null> returns null with strictNullChecks: false

🙂 Expected behavior

NonNullable<null> always returns never

[DanielRosenwasser]

To be honest, I think that the behavior outside of strictNullChecks wasn't well-considered here - but in part because it's hard for us to imagine how this caused a meaningful change in type-checking behavior. Can you explain how this broke user code? sveltejs/kit#6387 doesn't give a lot of detail/context.

[fatcerberus]

@DanielRosenwasser I was skeptical about this at first too, but notably, null is not assignable to never — even under strictNullChecks: false — but is assignable to NonNullable<null> with strictNullChecks off (in 4.8). I could imagine that hiding some errors that would otherwise be caught.

[DanielRosenwasser]

Well... it probably should be the case that null and undefined are assignable to never outside of strictNullChecks.

[DanielRosenwasser]

I'm going to regret that comment I bet.

[fatcerberus]

Regardless of whether anyone feels null should be assignable to never or not, it currently isn't:
https://www.typescriptlang.org/play?strictNullChecks=false&ts=4.8.0-beta#code/DYUwLgBAHgXBB2IBuIBOEC8CCuxgG4AoUSATzkRXS23gBMQAzAS0TqJIgC84A5Ae3i9cwAIYAjUAB54IgHyYceIkA

[dummdidumm]

Can you explain how this broke user code?

It broke because we use NonNullable in combination with a helper type to either resolve to type X or Y that looks something like this:

export type EnsureDefined<T> = NonNullable<T> extends never ? {} : T;

We need this because {} & null === null, but in our situation we have some types that should be merged and null should not make the whole object go null, instead it should be like a noop. Therefore we do something like EnsureDefined<X> & EnsureDefined<Y>. This worked with 4.7 and broke with 4.8. It's not tragic because we can adjust this easily but there might be other helper types out there relying on this behavior being consistent whether or not strict null checks are enabled.

[ahejlsberg]

NonNullable<null> and NonNullable<undefined> should resolve to never as they used to. Which of course implies that null & {} and undefined & {} should resolve to never.

[fatcerberus]

By the rules of assignability, shouldn’t null & {} be null without strictNullChecks? NonNullable<null> should definitely be never but the intersection case seems less clear-cut.

[DanielRosenwasser]

It is all pretty hard to reason about, and I mixed it up a few times - but null & {} needs to be null since that is the type that is compatible with both constituents. {} is assignable to nothing else, whereas null and undefined are assignable to everything else (meaning they're the bottom types and are sort of equivalent to never).

[ahejlsberg]

In 4.8 NonNullable<T> is defined as T & {}, so there's no distinction between the two. In non-strictNullChecks mode, the set of possible values for null is null or undefined, and the set of possible values for {} is anything, including null and undefined. The intersection of these sets is null and undefined, which argues that null & {} should be null. However, the pre-4.8 definition of NonNullable<T> as a conditional type produced never for null and undefined, so in this PR we're aligning with that behavior and producing never for null & {}. I think that's perfectly fine, particularly since the non-strictNullChecks behavior isn't really reflecting reality anyway.