Skip to content

Issues with parameterizing over a union type #13073

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

Closed
johnfn opened this issue Dec 20, 2016 · 13 comments
Closed

Issues with parameterizing over a union type #13073

johnfn opened this issue Dec 20, 2016 · 13 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@johnfn
Copy link

johnfn commented Dec 20, 2016

Why does this code work:

type KeyTypes = "a" | "b"
let MyThingy: { [key in KeyTypes]: string[] };

function addToMyThingy(key: "a" | "b") {
    MyThingy[key].push("a");
}

And this code fail?

type KeyTypes = "a" | "b"
let MyThingy: { [key in KeyTypes]: string[] };

function addToMyThingy<S extends KeyTypes>(key: S) {
    MyThingy[key].push("a"); // Error: Property 'push' does not exist on type '{ a: string[]; b: string[]; }[S]'.
}

Unless I've misunderstood S extends KeyTypes, all I've done is constrained key to be "a" or "b" with my type parameter.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

if your function expects a value that is either "a" | "b", and the result does not depend on which one you get then use the concreate type for your parameter, and not a generic.

if your result depends on which value you pass, e.g. return MyThing[Key], then use the generic type. Keep in mind that the constraint is useful for specifying bounds but the type parameter is still a generic type.

The evaluation of the constraint for generic indexed access type is delayed until instantiation time. doing it earlier will allow you to access the push method, in the example below, but will also have the side effect of "fixing" the type of the the indexed access MyThing[key], which is not desired.

For instance:

type KeyTypes = "a" | "b"
let MyThingy: {
    a: string;
    b: number;
};

function genericGet<S extends KeyTypes>(key: S) {
    return MyThingy[key]; 
}

var g = genericGet("b"); // number

function concreateGet(key: KeyTypes) {
    return MyThingy[key]; 
}

var c = concreateGet("b"); // string | number

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Dec 21, 2016
@johnfn
Copy link
Author

johnfn commented Dec 21, 2016

I see what you're saying about the constraints being delayed until instantiation. But isn't it possible to determine that MyThingy[key] is an array no matter what the type of key, even at compile time? Since we know that S extends KeyTypes?

Unfortunately, I really do need the generic type. This is a minimum reproducible example of a much larger project.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

if you can use it as an array then the result will be the union type as well. do not think return MyThingy[key]; should return MyThing["a"] | MyThing["b"], you really want it to be MyThing[key]

@dtabuenc
Copy link

dtabuenc commented Dec 21, 2016

This might be more representative of what he's asking:

interface Foo{
    prop1: string;
    prop2: number;
}
type PropertyEditCount = {
     [key in keyof Foo]: number
}

const propEditCount: PropertyEditCount = {
    prop1: 0,
    prop2: 0
}

const foo:Foo = {
    prop1: 'bar',
    prop2: 42
}

class FooPropertyEditor<T extends keyof Foo>{
    setVaue(key: T, value: Foo[T]): void{
        foo[key] = value;
        propEditCount[key] = propEditCount[key]+1;//Operator + cannot be applied to types PropertyEditCount[T] and '1'
    }
}

Why can't propEditCount[key] be resolved to a number at compile time seems like all the information is there.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

for +, this should be handled by #12410. the rule on + should be relaxed to allow for type variables (that is a type parameter or an index type) with constraints that resolve to string | number.

@dtabuenc
Copy link

I don't think that's the crux of it. The fundamental question is why the parameterized is not being fully resolved at compile time. Take a slightly different example without +:

interface Foo{
    prop1: string;
    prop2: number;
}
type DateEditedCount = {
     [key in keyof Foo]: Date[]
}

const propEditCount:  DateEditedCount= {
    prop1: [],
    prop2: [] 
}

const foo:Foo = {
    prop1: 'bar',
    prop2: 42
}

class FooPropertyEditor<T extends keyof Foo>{
    setVaue(key: T, value: Foo[T]): void{
        foo[key] = value;
        propEditCount[key] = propEditCount[key].push(new Date()); //ERROR Property 'push' does not exist on type 'DateEditedCount[T]'
    }
}

@dtabuenc
Copy link

In that last example we know that T is limited to keys of Foo, and that DateEditedCount maps all keys of Foo to Date[] so when we do DateEditedCount[T] we should be able to resolve that at compile time to a Date[].

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

this is a special case, where the type of propEditCount[prop1] | propEditCount[prop2] is Date[]. it is not the same in the general case where the types are different. the options here either a. you get union type, or b. an indexed access type e.g. T[K], and a thing can not have two types.. a. is clearly wrong, since you want T[K] to be the K that the generic function was instantiated with, and not a union of all the possible values of K.

@dtabuenc
Copy link

dtabuenc commented Dec 21, 2016

It is not uncommon to define a type that maps all keys of another type to a common type. I don't know anything about the compiler implementation or complexities involved, but would it be worth supporting this case such that if the union of all the possible T[K] is all the same, it just maps to the common type?

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

the issue is not the mapped type. it is the index access with a generic type, i.e. propEditCount[key].

you could define propEditCount as { [x: string]: Date[] }, or define DateEditedCount as { [key in keyof Foo]: Date[] } & { [x: string]: Date[] };

@dtabuenc
Copy link

Ok.. I think I have a better understanding of the issue now.. Thanks for your help.

@ahejlsberg
Copy link
Member

Returning to the original question, I do think we could do better.

interface Item {
    a: string;
    b: string;
}

function f1(x: Item[keyof Item]) {
    x.length;  // Ok
}

function f2<T extends Item>(x: T[keyof Item]) {
    x.length;  // Ok
}

function f3<K extends keyof Item>(x: Item[K]) {
    x.length;  // Error
}

function f4<T extends Item, K extends keyof Item>(x: T[K]) {
    x.length;  // Error
}

In the two error cases above we know from the constraint that K will be a subtype of 'a' | 'b' and therefore x will be a subtype of string. Similarly to how we say for a type parameter T extends A the apparent type of T[K] is A[K] (as demonstrated by f2 above), we could say that for a type parameter K extends B, the apparent type of T[K] is T[B]. That would fix f3. Combining the two would fix f4.

@ahejlsberg ahejlsberg added Bug A bug in TypeScript and removed Question An issue which isn't directly actionable in code labels Jan 7, 2017
@ahejlsberg ahejlsberg self-assigned this Jan 7, 2017
@ahejlsberg ahejlsberg added this to the TypeScript 2.2 milestone Jan 7, 2017
@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Jan 10, 2017
@johnfn
Copy link
Author

johnfn commented Jan 10, 2017

Whoa, awesome! Thanks!

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants