Skip to content

feat: Nullable<T> lib type (Taylor's Version) #51254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

ctjlewis
Copy link
Contributor

@ctjlewis ctjlewis commented Oct 21, 2022

There is already a NonNullable<T>lib type, which does not seem specified in ECMA-262 and so should not be a valid basis for rejecting this PR, which I am sure many people will be thankful for as a good chunk of TS workspaces define this manually, usually as the T | null union.

This suggestion was resisted in #19944 (and many, many subsequent PRs and issues), presumably because it's hard to know whether to use T | null or the example provided under the old handbook, { [P in keyof T]: T[P] | null } (all properties on T assignable to null).

There is nothing that stops us from doing both, and constructing a utility type Nullable<T> which can be assigned either to null or T with all properties assignable to null:

type Nullable<T> = null | { [P in keyof T]: null | T[P] };

This is not an existing utility type and so it is not as if it's breaking. It is not equivalent to T | null but is a superset of it, so there is more utility baked in than you can describe with T | null while still getting equivalent behavior for primitives. This would afford a lot of convenience without any apparent drawbacks as far as I can tell, and the other PRs seem to have been closed mostly on the basis of vibes only, despite lots of pushback in the RFC and several issues/PRs opened between 2016 and now.

Ultimately it does seem kind of ridiculous to leave a NonNullable<T> type without defining a Nullable<T> one, and I suggest we take a stab at it.

Fixes: #51255

There is already a `NonNullable<T>`lib type, which does not seem specified in ECMA-262 and so should not be a valid basis for rejecting this PR, which I am sure many people will be thankful for as [a good chunk](https://www.google.com/search?q=%22Nullable%3CT%3E%22+includes%3Atypescript) of TS workspaces define this manually, usually as the `T | null` union.

This suggestion was resisted in microsoft#19944, presumably because it's hard to know whether to use `T | null` or the example provided under the [**old handbook**](https://www.typescriptlang.org/docs/handbook/advanced-types.html#mapped-types), `{ [P in keyof T]: T[P] | null }` (all properties on `T` assignable to `null`).

There is nothing that stops us from doing both, and constructing a utility type `Nullable<T>` which can be assigned either to `null` *or* `T` with all properties assignable to `null`:

```ts
type Nullable<T> = null | { [P in keyof T]: null | T[P] };
```

This is not an existing utility type and so it is not as if it's breaking. This would afford a lot of convenience without any apparent drawbacks as far as I can tell, and the other PR seems to have been closed mostly on the basis of vibes only, not for any real reason.

Ultimately it does seem kind of ridiculous to leave a `NonNullable<T>` type without defining a `Nullable<T>` one, and I suggest we take a stab at it.
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 21, 2022
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@ctjlewis
Copy link
Contributor Author

ctjlewis commented Oct 21, 2022

I submitted this lazily through GitHub GUI but I'll fill out the lib sheet if someone confirms they're not just going to throw this in the garbage, which I'm expecting.

I know this to not be correct given the versatility of this approach and also the high occurrence of this utility type (usually T | null variant) in real-world codebases:

The conclusion a while back in #7426 was that | null and | undefined explicitly was the best way forward. Our experience since has validated that conclusion. the two extra types are not too long to write, nor they are hard to understand; and thus the additional layer of indirection using a type alias seems to be unwarranted here.

Again, if you find these aliases helpful, you are more than welcome to define them locally and use them. our experience however does not suggest there is a need for them in the wider community at the time being.

Originally posted by @mhegazy in #19944 (comment)

@MartinJohns
Copy link
Contributor

MartinJohns commented Oct 21, 2022

This was declined already. See #39522.

Besides that, I find it very confusing that NonNullable<T> includes undefined, but your suggestion for Nullable<T> does not. Additionally, NonNullable<T> operates on T, while your Nullable<T> operates on the properties of T.

Also, this is a breaking change for everyone who might already have a global Nullable<T> type declared. A painful one for everyone who has a different implementation than your suggested one.

@Jack-Works
Copy link
Contributor

If Nullable only includes null, it will not useful enough. I'd like to include undefined to make it useful

@DanielRosenwasser
Copy link
Member

A few things.

First, we generally don't want to add more helper types to lib.d.ts if they won't be used as part of type-checking/declaration emit.

Second, it's been years since #19944 was filed, and I've not seen enough evidence that having an "inverse" of NonNullable is a huge stumbling block. In fact, not adding these aliases (or special syntax like ? to imply | null | undefined) is a design decision that has sat with us very well.

Third, the way this works is not at all how I, nor probably anyone else on the team, would define it. You already have several differing opinions of how this should work, which is what we already experienced in the early discussions around nullability. If you add a feature like Nullable, people will expect that they should use it -- and if it works counter to the expectations of half the people who try to use it, you're shipping an anti-feature.

Finally

I submitted this lazily through GitHub GUI but I'll fill out the lib sheet if someone confirms they're not just going to throw this in the garbage, which I'm expecting.

this is another warning - your interaction on the issue tracker is regularly hostile. You cannot keep throwing the maintainers under the bus when you disagree with a design decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NonNullable<T> type does not have a corresponding Nullable<T> type
5 participants