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

RequiredProps<T>: Unexpected result when T = {} #21988

Closed
yortus opened this issue Feb 16, 2018 · 6 comments
Closed

RequiredProps<T>: Unexpected result when T = {} #21988

yortus opened this issue Feb 16, 2018 · 6 comments

Comments

@yortus
Copy link
Contributor

yortus commented Feb 16, 2018

TypeScript Version: 2.8.0-dev.20180216

Code

// Type-level filters to extract just the required or optional properties of a type
// Defns from https://github.com/Microsoft/TypeScript/pull/21919#issuecomment-365491689
type RequiredPropNames<T> = { [P in keyof T]: undefined extends T[P] ? never : P }[keyof T];
type OptionalPropNames<T> = { [P in keyof T]: undefined extends T[P] ? P : never }[keyof T];
type RequiredProps<T> = { [P in RequiredPropNames<T>]: T[P] };
type OptionalProps<T> = { [P in OptionalPropNames<T>]: T[P] };

// Some object types with different numbers of props
type P2 = {a: string, b: number};       // Two props
type P1 = {a: string};                  // One prop
type P0 = {};                           // No props

// Let's extract only the required properties of P0, P1, and P2
type P2Names = RequiredPropNames<P2>;   // P2Names = "a" | "b"                  ✓😊
type P1Names = RequiredPropNames<P1>;   // P1Names = "a"                        ✓😊
type P0Names = RequiredPropNames<P0>;   // P0Names = any                        ?😕

type P2Props = RequiredProps<P2>;       // P2Props = { a: string; b: number; }  ✓😊
type P1Props = RequiredProps<P1>;       // P1Props = { a: string; }             ✓😊
type P0Props = RequiredProps<P0>;       // P0Props = { [x: string]: any }       ?😕

Expected behavior:
P0Names = never and P0Props = {}

Actual behavior:
P0Names = any and P0Props = {[x: string]: any}

Notes:
@ahejlsberg helpfully came up with the RequiredProps and OptionalProps definitions above from a question I asked in #21919. They work great except when T = {}

Not 100% sure if this is a bug or not, but it's definitely counterintuitive. RequiredProps works for any object type except the empty one, when a string indexer suddenly appears. It certainly breaks the semantic expectations of RequiredProps.

Workaround:
The following version of RequiredPropNames achieves the expected behaviour by singling out the {} case:

type RequiredPropNames<T> =
    keyof T extends never
        ? never
        : {[P in keyof T]: undefined extends T[P] ? never : P}[keyof T];
@falsandtru
Copy link
Contributor

This means {}[never] should return never.

@yortus
Copy link
Contributor Author

yortus commented Feb 17, 2018

@falsandtru I agree. Actually I think T[never] should return never for any type T.

Index access notation T[K] is valid for all K extends keyof T. Given that never extends keyof T for all possible types T, then T[never] should always be a valid index access type. Its result should always be never, since the union of zero keys (never) maps to the union of zero values (never).

@yortus
Copy link
Contributor Author

yortus commented Feb 17, 2018

In #11929 (the PR introducing indexed access types) it states:

An indexed access type T[K] requires K to be a type that is assignable to keyof T

...and in #8652 (the 'never' PR) it states that "never is a subtype of and assignable to every type."

If K is a union type K1 | K2 | ... | Kn, T[K] is equivalent to T[K1] | T[K2] | ... | T[Kn]

...and if n=0, then we effectively have an empty union / union of zero types, which is the type having zero values, which is never.


Well that's my reasoning anyway. #11929 doesn't mention never, but I believe the reasoning here is consistent with the given type rules.

@weswigham
Copy link
Member

weswigham commented Feb 18, 2018

cc @sandersn didn't we recently change how T[any] behaved? Did we just leave out T[never]?

@yortus
Copy link
Contributor Author

yortus commented Feb 20, 2018

Closing in favour of #22042, which more simply addresses just the underlying issue.

@yortus yortus closed this as completed Feb 20, 2018
@sandersn
Copy link
Member

@weswigham I removed the constraint of T[string], which used to be any for unconstrained T. Maybe we have some more recent changes than that though?

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants