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

Bug in Pick and Discriminated Unions #28791

Closed
cliedeman opened this issue Dec 1, 2018 · 7 comments
Closed

Bug in Pick and Discriminated Unions #28791

cliedeman opened this issue Dec 1, 2018 · 7 comments

Comments

@cliedeman
Copy link

@cliedeman cliedeman commented Dec 1, 2018

TypeScript Version: 3.2.1

Search Terms: Discriminated Unions, Pick, Exclude

interface BaseTextFieldProps {
    name?: string
}

export interface StandardTextFieldProps extends BaseTextFieldProps {
    variant?: 'standard';
}

export interface FilledTextFieldProps extends BaseTextFieldProps {
    variant: 'filled';
}

export type TextFieldProps = StandardTextFieldProps | FilledTextFieldProps;

export type FieldProps = {
    someProps: string
};

export type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>;

export type Props1 = FieldProps & Omit<TextFieldProps, 'name'>

export const func1 = ({...props}: Props1): TextFieldProps => {

    return {
        ...props,
    };
};

export type Props2 = FieldProps & TextFieldProps

export const func2 = ({...props}: Props2): TextFieldProps => {

    return {
        ...props,
    };
};

Expected behavior:
Compiles

Actual behavior:
Errors with incompatible types for variant

Playground Link:

Related Issues:
#27201 (comment)
#28274
but these were related to generics so I could not be sure if this is a duplicate or not

@ahejlsberg

This comment has been minimized.

Copy link
Member

@ahejlsberg ahejlsberg commented Dec 1, 2018

This is working as intended. You are using Omit to remove the name property from TextFieldProps so the resulting type is no longer assignable to TextFieldProps (because the name property is missing).

@cliedeman

This comment has been minimized.

Copy link
Author

@cliedeman cliedeman commented Dec 1, 2018

Hi, Thanks for the reply but I don't quite understand. The error is not related to name but to variant?

@ahejlsberg

This comment has been minimized.

Copy link
Member

@ahejlsberg ahejlsberg commented Dec 1, 2018

@cliedeman Please post a complete repro. The current example starts with two undefined types.

@cliedeman

This comment has been minimized.

Copy link
Author

@cliedeman cliedeman commented Dec 1, 2018

Updated

@ahejlsberg

This comment has been minimized.

Copy link
Member

@ahejlsberg ahejlsberg commented Dec 2, 2018

Ah, I see what's going on. The issue is that Pick doesn't distribute over union types, and therefore your Omit type doesn't either. Here's a simplified version of your example:

type A = { name?: string, variant: 'a' };
type B = { name?: string, variant: 'b' };
type C = A | B;
type X = Omit<C, 'name'>;  // { variant: 'a' | 'b' }

Because Pick isn't distributive over union types, when the input type is a union it is effectively treated as a single object type with only the common properties, each having a union of the respective property types. So, Pick sees C as equivalent to { name?: string, variant: 'a' | 'b' } and X therefore is { variant: 'a' | 'b' }, which isn't assignable back to C.

There are two ways you can fix the issue. The simplest is to declare C as a single object type (and this would be my recommendation):

type C = { name?: string, variant: 'a' | 'b' };

The other is to make Omit be distributive over union types:

export type AllKeys<T> = T extends T ? keyof T : never;
export type Omit<T, K extends AllKeys<T>> = T extends T ? Pick<T, Exclude<keyof T, K>> : never;

This may look a bit odd, but the T extends T ? XXX : never pattern simply means "when T is a union type, distribute XXX over each constituent of T and union together the results".

When you use the distributive form of Omit you now get

type A = { name?: string, variant: 'a' };
type B = { name?: string, variant: 'b' };
type C = A | B;
type X = Omit<C, 'name'>;  // { variant: 'a' } | { variant: 'b' }

and X now is assignable back to C.

For the reason why we don't treat the two forms of X as equivalent, see my comment in #12052.

@que-etc

This comment has been minimized.

Copy link

@que-etc que-etc commented Dec 3, 2018

@ahejlsberg I have a somewhat related issue and I wonder if it's worth opening a separate ticket.

In the following example, when excluding some keys from a generic type, the resulting type doesn't contain properties which are meant to be there according to the base type constraint:

function excludeTag<T extends { tag: string, foo: string }>(obj: T) {
    let { tag, ...rest } = obj;
    let manual: Pick<T, Exclude<keyof T, "tag">>

    // 'foo' doesn't exist
    rest.foo;

    // 'foo' doesn't exist
    manual.foo;

    return rest;
}

const result = excludeTag({
    tag: 'point',
    foo: 'bar'
});

// 'foo' exists
result.foo;

Playground.

Please, note that the manually created type used to work in v3.1.6.

Edit: I feel like #28752 is also somehow related.

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

@typescript-bot typescript-bot commented Dec 13, 2018

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.