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

in typescript 2.8, a common definition of Omit causes declarations that don't compile to be generated #25041

Open
blushingpenguin opened this issue Jun 18, 2018 · 3 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@blushingpenguin
Copy link

TypeScript Version:
Reproduces with 3.0.0-dev.201xxxxx, 2.9.2, 2.8.4

Search terms
omit, declaration, pick, undefined

Example

export type Omit<T, K extends keyof T> = Pick<T, 
    ({ [P in keyof T]: P } & { [P in K]: never } )[keyof T]>;

export interface IOmitTest {
	(): { notSupposedToHappen: Omit<IXProps, "unwantedProp"> }
}

export interface IXProps {
    optionalProp?: string
    unwantedProp: string
}

const Y: IOmitTest = null as any;
export const Z = Y();

export interface IMouseOver {
    wrong: Omit<IXProps, "unwantedProp">
}

generates

export declare type Omit<T, K extends keyof T> = Pick<T, ({
    [P in keyof T]: P;
} & {
    [P in K]: never;
})[keyof T]>;
export interface IOmitTest {
    (): {
        notSupposedToHappen: Omit<IXProps, "unwantedProp">;
    };
}
export interface IXProps {
    optionalProp?: string;
    unwantedProp: string;
}
export declare const Z: {
    notSupposedToHappen: Pick<IXProps, "optionalProp" | undefined>;
};
export interface IMouseOver {
    wrong: Omit<IXProps, "unwantedProp">;
}

The interface returning a function seems to be required to get typescript to collapse the Omit to a Pick statement (otherwise you just get Omit<IXProps, "unwatedProp"> in the declaration file).

I think this is a bug in typescript because it's produced a declaration file with syntax errors in it.

Expected behavior:
undefined should not be present in the Pick statement

Playground Link:
the playground does not provide the ability to generate declartions

Related Issues:
#12215

note that if the definition of Omit is replaced with the newer declaration possible in typescript 2.9

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

then

export declare const Z: {
    notSupposedToHappen: Pick<IXProps, "optionalProp">;
};

results as expected.

I actually encountered this issue using when using connect() in react-redux with a component with an optional property, where the definition of Omit is:

type Omit<T, K extends keyof T> = Pick<T, ({ [P in keyof T]: P } & { [P in K]: never } & { [x: string]: never, [x: number]: never })[keyof T]>;
@mhegazy mhegazy added this to the TypeScript 3.0 milestone Jun 20, 2018
@mhegazy mhegazy added Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files labels Jun 20, 2018
@weswigham
Copy link
Member

weswigham commented Sep 5, 2018

export type Omit<T, K extends keyof T> = Pick<T, 
    ({ [P in keyof T]: P } & { [P in K]: never } )[keyof T]>;

contains a mapped type which is unintentionally homomorphic - a mapped type templated over keyof T, which thereby causes us to inherit the optionality of the properties of T in the resulting type. This means that for a given key P, if it is optional in T, it will be optional in { [P in keyof T]: P }, resulting in the type P | undefined for the property (for some specific P). Pick<T, "optionalProp" | undefined> is an error when written because we validate the constraints at that point in time, however when we check the constraint of { [P in keyof T]: P }[keyof T] we neglect to consider the possibility that undefined is propagated into the type! (Instead we simplify to just P instead of P | undefined)

For the OP, to fix that definition of omit, you should write:

export type Omit<T, K extends keyof T> = Pick<T, 
    ({ [P in keyof T]-?: P } & { [P in K]: never } )[keyof T]>;

to explicitly strip the input optionality (note the -? in the type).

For us: We should issue an error on

export type Omit<T, K extends keyof T> = Pick<T, 
    ({ [P in keyof T]: P } & { [P in K]: never } )[keyof T]>;

as the homomorphic mapped type { [P in keyof T]: P }[keyof T] should be constrained to P | undefined and not simply P (which is clearly not compatible with the constraint on Pick's second argument.

@weswigham
Copy link
Member

weswigham commented Sep 6, 2018

cc @ahejlsberg to whom I was talking about this earlier today. Fixing this by including undefined in the constraint of all indexes on all homomorphic mapped types is... breaky. In our own test suite, even. It would prevent assigning a Readonly<T>[K] to a T[K] because the constraint of a Readonly<T>[K] would simplify to T[K] | undefined (since Readonly is a homomorphic mapped type) instead of the T[K] it simplifies to today. That alone is a uuuuuge problem. I have a branch with what I've been looking at, and it's not pretty. 😦

@weswigham
Copy link
Member

@RyanCavanaugh we need to consider the implications of fixing this one.

tdeekens pushed a commit to tdeekens/flopflip that referenced this issue Dec 3, 2018
#### Summary

Updates some typings that cause build errors on `tsc` in Typescript v3.2.1
#### Description

- new Omit is from microsoft/TypeScript#25041 (comment)
- stops implicity 'any' error on next parameter
- fixes destructuring confusion
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript and removed Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Suggestion An idea for TypeScript labels Mar 13, 2019
@RyanCavanaugh RyanCavanaugh removed this from the TypeScript 3.4.0 milestone Mar 13, 2019
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Mar 13, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants