Skip to content
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

Distinguish missing and undefined object properties. #30796

Closed
wants to merge 10 commits into from

Conversation

jack-williams
Copy link
Collaborator

Starts the ball rolling on #13195.

A couple of things to highlight:

I looked into distinguishing optional function arguments too, but I got buried under a mountain of errors in checker.ts. I limited this change to just object properties, which I think is far more useful.

There is some interesting behaviour with index access type now, because they type property access. For example:

type A = { a?: number } 
type B = Pick<A, keyof A>; // { a?: number | undefined }
declare const a: A;
const b: B = a;
const c: A = b; // error

So now B is not assignable to A, when it was before.

@jack-williams
Copy link
Collaborator Author

So a couple of observations after playing whack-a-mole with undefined.

I think this change should come paired with some utility package to transform all existing properties:

{ foo?: Type } ~~~> { foo?: Type | undefined } // if undefined not in Type

In theory this should be semantics preserving and allow people to just 'upgrade' without deciding to spend a Saturday doing it manually like some idiots.

I don't think this will catch many existing correctness bugs. In most cases the old behaviour was conservative and prevented anything bad happening. The existing mechanisms that can actually distinguish missing or undefined properties seem rarely used. This comes with a big caveat: I'm not someone that uses in or hasOwnProperty a lot (if at all), so I might be being naïve here.

Where I think this change does shine is in preventing/understanding performance bugs that arise by messing up your object layouts. Assuming legacy properties have been transformed so that they look like:

interface Foo {
    x?: number | undefined;
}

it is now a one character change to track down the places where you don't initialise x explicitly and produce a different shape. If you only have a single location where x is missing then it seems better to change that and produce a consistent shape, than have x be optional, IMO.

@leighman
Copy link

leighman commented Apr 8, 2019

Do you plan to add a missing keyword to allow use in conditional types as #13195 (comment)

@jack-williams
Copy link
Collaborator Author

@leighman As it stands, no I don't plan to. My reason is that conditional types only relate types but missing is not really the type of a value, but an attribute of a type. When you access a missing property you really do just get back undefined, not some distinguished missing value.

I don't claim ownership of this feature or PR, so don't take this as a statement of this is how the feature is going to be. This is just me saying: please don't wait for me to add this.

@RyanCavanaugh RyanCavanaugh added the Experiment A fork with an experimental idea which might not make it into master label Apr 23, 2019
@ExE-Boss
Copy link
Contributor

@leighman The missing type you’re looking for already exists in the form of never.

@jplatte
Copy link

jplatte commented Oct 30, 2019

@ExE-Boss I'm pretty sure never doesn't help at all here. For example

interface X { field: never; }
const x: X = {};

doesn't work. A missing type or keyword would be useful, for example, in a type where a group of fields are optional, but if one is given, all of them must be given. With missing, this could be implemented as

type AllFieldsMissing<T> = {
    missing [P in keyof T];
    // or
    //[P in keyof T]: missing;
};

type AllFieldsOrNone<T> = T | AllFieldsMissing<T>;

type MyType = MyBaseType & AllFieldsOrNone<ExtraFieldGroup>;

interface MyBaseType {
    a: number;
    b: string;
}

interface ExtraFieldGroup {
    c: string;
    d: 'some thing' | 'other thing';
    e: number;
}

I'm pretty sure this can't be implemented yet.

@sandersn sandersn added this to Bête Noire in Pall Mall Jan 30, 2020
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experiment A fork with an experimental idea which might not make it into master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants