-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Regression when using indexed type depending on function argument as return type #31672
Comments
I'm thinking this is probably a result of #30769. |
I'm not sure if this is a bad thing at all. More inconvenient, for sure. But if you're sure it's correct, I think having an explicit type assertion ( |
Yeah, the new behavior is in general much more sound. In this particular case the code looks okay since the Too bad there wasn’t a way to make the compiler narrow type parameters like we can narrow variable and property types... |
This wasn't properly checked in 3.4: interface Foo {
prop1: { value: number }[];
prop2: string[];
}
function getFoo<T extends keyof Foo>(key: T): Foo[T] | undefined {
if (key === "prop1") {
// Oops!
return ["foo"];
}
}
const bar = getFoo("prop1"); |
Yep, might be nice if there was a way to narrow whole type parameters based on type guards, but that kind of thing is probably really challenging to implement in practice. Particularly in cases where there's more than one inference site for a type. |
You can't narrow a type parameter based on a value, because you can't know that the type parameter isn't wider than the value you saw: interface Foo {
prop1: { value: number }[];
prop2: string[];
}
function getFoo<T extends keyof Foo>(key1: T, key2: T): Foo[T] | undefined {
// checking key1 cannot tell you what key2 will be or what a valid return type is
return null as any;
}
// T: "prop1" | "prop2"
const bar = getFoo("prop1", "prop2"); |
Even limiting it to types with one inference site is insufficient because one site can produce multiple values: Eg. |
@RyanCavanaugh Don't you think cases like this should work though? Is this unsound? interface ConfigMap {
Number: number;
String: string;
Boolean: boolean;
}
const MyNumber: number = 0;
const MyString: string = "foo";
const MyBool: boolean = false;
export function GetConfiguration<K extends keyof ConfigMap>(key: K): ConfigMap[K] {
if (key === "Number") {
return MyNumber;
} else if (key === "String") {
return MyString;
} else if (key === "Boolean") {
return MyBool;
}
}
const a = GetConfiguration("Boolean");
const b = GetConfiguration("Number");
const c = GetConfiguration("String"); |
Counterexample: function GetConfiguration<K extends keyof ConfigMap>(fakeKey: K, realKey: K): ConfigMap[K]
{
if (fakeKey === "Number") {
return MyNumber;
} else if (fakeKey === "String") {
return MyString;
} else if (fakeKey === "Boolean") {
return MyBool;
}
} The above code will be accepted by TS 3.4, but not by 3.5. Basically the problem is this:
In the case where all conditions above are met, the code is sound. The compiler can prove 1 and 2, but without dependent types it can't prove 3, and because this is unsafe more often than not, you get a type error to warn you of that. This is an example of contravariance at work, so I can see why it throws people off (contravariance is confusing!) - but it's there for a good reason 😄 |
@fatcerberus I am not sure whether it is "unsafe more often than not", but I don't dispute that a case like the example you gave should make typescript error. I simply think it would be nice if TypeScript allowed cases where the types are sound, and restricted them when they are unsound. |
That's the point though - the compiler has no way to prove that it's safe (that is, it can't distinguish the safe cases from the unsafe ones). In my checklist above, it's only safe when all 3 of those conditions are met, and the compiler can't prove the third condition without dependent types. So the choice you have is either to let all the unsafe cases through (i.e. any case not meeting the 3 conditions, such as the code above), or restrict it entirely at the cost of a few specific examples. |
Proposal: introduce one-of type parameter constraint. declare function foo<T extends oneOf 1|2|3> (t : T) : void;
foo(1); //OK
foo(2); //OK
foo(3); //OK
foo(1 as 1|2); //Error |
You can sort of already force users to always pass in a single string literal that is not a union of other types, type IsAny<T> = (
0 extends (1 & T) ?
true :
false
);
type IsNever<T> = (
[T] extends [never] ?
(
IsAny<T> extends true ?
false :
true
) :
false
);
type IsStringLiteral<T extends string> = (
IsAny<T> extends true ?
false :
IsNever<T> extends true ?
false :
string extends T ?
false :
true
);
type IsOneStringLiteral<T extends string> = (
IsStringLiteral<T> extends true ?
(
{
[k in T] : (
Exclude<T, k> extends never ?
true :
false
)
}[T]
) :
false
);
//false
type a = IsStringLiteral<any>;
//Err: Type 'unknown' does not satisfy the constraint 'string'.ts(2344)
type b = IsStringLiteral<unknown>;
//false
type c = IsStringLiteral<never>;
//false
type d = IsStringLiteral<string>;
//true
type e = IsStringLiteral<"x">;
//true
type f = IsStringLiteral<"x"|"y">;
//false
type _a = IsOneStringLiteral<any>;
//Err: Type 'unknown' does not satisfy the constraint 'string'.ts(2344)
type _b = IsOneStringLiteral<unknown>;
//false
type _c = IsOneStringLiteral<never>;
//false
type _d = IsOneStringLiteral<string>;
//true
type _e = IsOneStringLiteral<"x">;
//false
type _f = IsOneStringLiteral<"x"|"y">;
interface ConfigMap {
Number: number;
String: string;
Boolean: boolean;
}
const MyNumber: number = 0;
const MyString: string = "foo";
const MyBool: boolean = false;
declare function GetConfiguration<K extends Extract<keyof ConfigMap, string>>(
key : (
IsOneStringLiteral<K> extends true ?
K :
[K, "is not a single string literal"]
)
): ConfigMap[K];
const a = GetConfiguration("Boolean"); //boolean
const b = GetConfiguration("Number"); //number
const c = GetConfiguration("String"); //string
/*
Argument of type '"Number" | "Boolean"' is not assignable to
parameter of type '["Number" | "Boolean", "is not a single string literal"]'.
Type '"Number"' is not assignable to
type '["Number" | "Boolean", "is not a single string literal"]'.ts(2345)
*/
const err = GetConfiguration("Boolean" as "Boolean"|"Number");
/*
Argument of type '"Number" | "String" | "Boolean"' is not assignable to
parameter of type '["Number" | "String" | "Boolean", "is not a single string literal"]'.
Type '"Number"' is not assignable to
type '["Number" | "String" | "Boolean", "is not a single string literal"]'.ts(2345)
*/
const err2 = GetConfiguration("Boolean" as Extract<keyof ConfigMap, string>); |
@RyanCavanaugh So if this is working as intended, is there any way to define the behavior I was trying to achieve here? So given the interface interface Foo {
prop1: { value: number }[];
prop2: string[];
} and a function with a single parameter with type
I assume this should be possible somehow without resorting to |
Here's a hack-y workaround that does not use interface Foo {
prop1: { value: number }[];
prop2: string[];
}
function getFoo<T extends keyof Foo>(key: T): Foo[T] | undefined {
/*
Works, but you must have a `Foo` instance
*/
const tmp : Foo = {
prop1 : [],
prop2: [],
};
if (key === "prop1") {
//I find it concerning that even strict equality (===)
//DOES NOT narrow `key` to type `"prop1"`.
//But I guess if it did narrow, then `tmp[key]` would fail again...
//Unless TS can somehow realize that `key` is both `"prop1"`
//and can still be used to satisfy `Foo[T]`
//Set tmp.prop1 to be the return value you want.
tmp.prop1 = [{value : 1}];
return tmp[key];
} else if (key === "prop2") {
//Set tmp.prop2 to be the return value you want.
tmp.prop2 = ["bye, world"];
return tmp[key];
} else {
return undefined;
}
}
const bar = getFoo("prop1"); // has correct type If you don't want to keep instantiating the I think using |
Other proposal, automatically narrow interface Foo {
prop1: { value: number }[];
prop2: string[];
}
function getFoo<T extends keyof Foo>(key: T): Foo[T] | undefined {
/*
Works, but you must have a `Foo` instance
*/
const tmp : Foo = {
prop1 : [],
prop2: [],
};
if (key === "prop1") {
//Can TS Make this narrowing automatically?
const k = key as (T & "prop1");
//Set tmp.prop1 to be the return value you want.
tmp[k] = [{value : 1}];
//This works
return tmp[k];
} else if (key === "prop2") {
//Can TS Make this narrowing automatically?
const k = key as (T & "prop2");
//Set tmp.prop2 to be the return value you want.
tmp[k] = ["bye, world"];
return tmp[k];
} else {
return undefined;
}
}
const bar = getFoo("prop1"); // has correct type |
I believe so too, at least compared to the workarounds you posted. |
@fatcerberus @RyanCavanaugh I agree that dependent types are the more sane solution to making these use cases work nicely, however I don't fully agree with:
or
Let's take a look at this example:
Indeed, checking Unfortunately this observation does not help us further in the general case with indexed access types as far as I can see. However, together with an addition such as the |
We have unions - |
Yes, |
I don’t think TypeScript currently has any way internally of representing a lower-bounded type. There was a proposal around here for a Yeah, here it is: #14520 |
Something like #27808 (or maybe #25879) would help here... or even #25051. Anything that lets you treat a value of a union type of n constituents like "exactly one of these n types" and not "one of all 2n possible sets of these n types". Lots of Stack Overflow questions about this issue are coming. |
I have an experimental PR (#30284) that starts on that track, but it would need explicit support for index access types. |
This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes. |
TypeScript Version: 3.5.1
Search Terms:
Code
Expected behavior:
No error. This used to work in 3.4.5
Actual behavior:
The text was updated successfully, but these errors were encountered: