microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.34k stars 12.4k forks source link

Record value type lost when key type is generic #40419

Closed dbartholomae closed 4 years ago

dbartholomae commented 4 years ago

TypeScript Version: 4.1.0-dev.20200907

Search Terms: Generic, Keys, Record

Code

type CustomRecord<Keys extends string> = Record<Keys, string>

function mapValues<Keys extends string> (entries: CustomRecord<Keys>): string[] {
  return Object.values(entries)
    .map((entry: string) => entry)
}

Expected behavior:

No error.

Actual behavior:

TypeScript gives the following error for the function given to map:

Argument of type '(entry: string) => string' is not assignable to parameter of type '(value: unknown, index: number, array: unknown[]) => string'. Types of parameters 'entry' and 'value' are incompatible. Type 'unknown' is not assignable to type 'string'.

Playground Link: https://www.typescriptlang.org/play?#code/C4TwDgpgBAwgrgZ2AewLYCUIGNkCcAmAPANIQgJQQAewEAdvhUrgJZ0DmAfFALxSY4CJMggA0UZmy4AoaQDM4dLMBbI6UVAEMwANU0AbOBATDylGvUYTgrDtwAU9Gy2MAuWIhQZseIqXKcAJTukhwA2gC6UADe0lBQuBDAcLjqAPIARgBW2MAAdABuBkYIjnTOxoFx8VB5WmD2ZTYgIc4cgbzcTrggVQC+0kA

Related Issues: None found

devanshj commented 4 years ago

That's becuase Object.values types are not accurate at all. Here's a better version:

type CustomRecord<Keys extends string> = Record<Keys, string>

const objectValues = <O extends object>(o: O): Array<O[keyof O]> =>
    Object.values(o) as any;

function mapValues<Keys extends string> (entries: CustomRecord<Keys>): string[] {
  return objectValues(entries)
    .map(entry => entry) // `entry` inferred as string
}

Playground

You might want to keep this issue open to improve the library types

RyanCavanaugh commented 4 years ago

This is the correct behavior; the function is not a correct implementation of its signature.

type CustomRecord<Keys extends string> = Record<Keys, string>

function mapValues<Keys extends string>(entries: CustomRecord<Keys>): string[] {
  return Object.values(entries).map((entry: string) => entry)
}

// Legal initialization
const obj = { a: 1, s: "" };
// Legal aliasing
const alias: CustomRecord<"s"> = obj;
// Legal call
const arr = mapValues(alias);
// Unsound, number inhabits string
const n: string = arr[0];

See also https://stackoverflow.com/questions/55012174/why-doesnt-object-keys-return-a-keyof-type-in-typescript

devanshj commented 4 years ago

Ah my bad forgot you could assign like that. I guess this is something hegel (afaict) mentions too as a disadvantage of TypeScript (second point in the link). I mean TypeScript has it's very rightful reasons to allow that it just reminded me of it hence linking haha. I personally would type it like because I don't see myself assigning like that :P

RyanCavanaugh commented 4 years ago

Well, in this case TypeScript is being sound by preventing you from writing unsound code...

dbartholomae commented 4 years ago

Thanks, this makes a lot of sense! I did not take into consideration that a Record with limited keys is actually weaker than a Record with all keys.

devanshj commented 4 years ago

Well, in this case TypeScript is being sound by preventing you from writing unsound code...

Agreed! Although I would say the way Object.values is typed is preventing not TypeScript per se. Allowing that assignment is still kinda a trade-off afaiu.

dbartholomae commented 4 years ago

After looking into it a bit more, I guess what I want would be something like:

type CustomRecord<Keys extends string> = { [key in Keys]: string }
  & Omit<{ [key in string]: never }, Keys>

A type for an object where every key from Keys is a string and no other key exists. This version doesn't work, but I don't really understand why yet. Will keep you posted once I figured out whether it's a bug, a feature request, or just me not understanding TypeScript well enough ;)

devanshj commented 4 years ago

Well that does not work is because you expect this to work ...

const allStringExceptA: Exclude<string, "a"> = "a" // no error

... that's because string is not really a set of all string literals (I mean it is but okay), so Exclude<string, "a"> evaluates to string. Or better to simply put: TypeScript doesn't have a type that expresses allStringExceptA (yet :P)

Also I would suggest using a Map if it's possible in your situation because then it would solve the problem of aliasing

type CustomRecord<Keys extends string> = Map<Keys, string>

function mapValues<Keys extends string> (entries: CustomRecord<Keys>): string[] {
  return Object.values(entries)
    .map(entry => entry)
}

const obj = new Map<"a" | "s", string | number>([["a", 1], ["s", ""]]);
const alias: CustomRecord<"s"> = obj; // not allowed

And I already know what it is haha, it's not a bug in Object.values ofc and a feature request if you somehow want to disallow that aliasing which would be a duplicate of #12936. And btw, I didn't have this issue, I was just going on round answering some issues haha.

Also I take my words back on how allowing that aliasing is a compromise, I was being silly, I realized that it indeed follows type theory (afaict) and should be allowed. It's because of the leaky things you can do in JavaScript like Object.values (and many more), Hegel goes kinda against the type theory (by default, you can still do it via $Soft).

I'll leave these few scenarios how both handle things in case someone is interested: 1, 2, 3, 4, 5, 6. Technically, Object.values should always return unknown[] in typescript or else it would result in unsound behavior like in 2, but probably typescript makes a little trade-off by not doing that when the key is string idk. And I LOVE TYPESCRIPT if it comes across otherwise ;)

typescript-bot commented 4 years ago

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.