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

Distribute 'keyof' union types #23626

Closed
wants to merge 4 commits into from
Closed

Conversation

ahejlsberg
Copy link
Member

With this PR we transform keyof applied to a union type to an intersection of keyof applied to each union constituent. In other words, we rewrite types of the form keyof (A | B) to keyof A & keyof B.

Some examples:

type A = { a: string, c: string };
type B = { b: string, c: string };

type T1 = keyof (A | B);  // "c"
type T2<T> = keyof (T | B);  // (keyof T & "b") | (keyof T & "c")
type T3<U> = keyof (A | U);  // ("a" | keyof U) | ("c" & keyof U)
type T4<T, U> = keyof (T | U);  // keyof T & keyof U
type T5 = T2<A>;  // "c"
type T6 = T3<B>;  // "c"
type T7 = T4<A, B>;  // "c"

This PR is the mirror image of #22300 and finally brings symmetry to the type eqivalences:

keyof (A & B) <==> keyof A | keyof B
keyof (A | B) <==> keyof A & keyof B

Why it took so long to figure out I don't know!

Fixes #23618.

# Conflicts:
#	src/compiler/checker.ts
#	tests/baselines/reference/keyofAndIndexedAccessErrors.errors.txt
#	tests/baselines/reference/keyofAndIndexedAccessErrors.symbols
@@ -12,8 +12,8 @@ class C extends Base {
>super : typeof Base

var obj = {
>obj : { [x: string]: () => void; }
>{ [(super(), "prop")]() { } } : { [x: string]: () => void; }
>obj : { [(super(), "prop")](): void; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I don't know if it's related to this PR, but this change caught my eye:

{ [(super(), "prop")](): void; }

isn't a valid type. Somewhere we're reusing the name's expression tree when building the computed property name type and shouldn't be. :(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a baseline change from #23592. I somehow messed up a merge and ended up with a lot of apparent changes in this PR.

Anyways, what's not valid about the type? It's a method with a computed property name that uses the comma operator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's supposed to be type output - if someone wrote type Foo = { [(super(), "prop")](): void; }, we'd issue an error.

@ahejlsberg
Copy link
Member Author

I somehow ended up with a bad merge on this branch so I'm abandoning this PR in favor of #23645.

@ahejlsberg ahejlsberg closed this Apr 24, 2018
@ahejlsberg ahejlsberg deleted the distributeKeyofUnion branch April 24, 2018 04:15
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 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

Successfully merging this pull request may close these issues.

4 participants