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

Cannot use matching indexes across objects with similar types #31445

Closed
ftrimble opened this issue May 17, 2019 · 11 comments
Closed

Cannot use matching indexes across objects with similar types #31445

ftrimble opened this issue May 17, 2019 · 11 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@ftrimble
Copy link

ftrimble commented May 17, 2019

TypeScript Version: Reproduced in the playground - i assume this is running latest. before running in the playground was running on 3.4.5

Search Terms:
keys across objects cannot be compared
type values at same indexes do not honor that keys can only assume one value at a time

(I'm having a hard time describing this issue, so I may have failed to search for it effectively; forgive me if that's the case.)

Code

type A = { a: 'a' | 'b'; b: 'c' | 'd'; };
type B = { a: ('a' | 'b')[]; b: ('c' | 'd')[]; };
type C = { a: string; b: number };
type D = { a: string[]; b: number[]; }


const a: A = { a: 'a', b: 'c' };
const b: B = { a: [], b: [] };
const c: C = { a: 'asdf', b: 1234 };
const d: D = { a: [], b: [] };

(['a', 'b'] as const).forEach(key => {
    b[key].push(a[key]);
    b[key] = [...b[key], a[key]];
    d[key].push(c[key]);
    d[key] = [...d[key], c[key]];
});

Expected behavior:

All of the options for adding the keys to the array in the second half of the example above should compile and work correctly.

Actual behavior:

The LHS of the equation is considered an & type while the RHS is an | type, which makes them incompatible. These types are clearly correct and should work, but e.g. b[key] is consider a never for the purposes of push ('a' | 'b' & 'c' |'d'), while d[key] is considered a string & number for those purposes (not sure why this isn't also a never)

The important part of this issue is that none of the possible ways of using these types together compile. All possible variants create too strict of a typing for what needs to go into the array variant of the original type, even though the types of the objects at identical keys are compatible

Playground Link:

playground link

Related Issues:

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 17, 2019

These errors are correct from the type system's perspective because there's no mechanism that correlates that both utterings of key must be the exact same value - they simply have the same type, which is not a sufficient proof that i.e. b[k1].push(a[k2]) is a valid call because k1 could be "a" and k2 could be "b"

The "expanded" form checks as expected:

(['a', 'b'] as const).forEach(key => {
    if (key === 'a') {
        b[key].push(a[key]);
        b[key] = [...b[key], a[key]];
        d[key].push(c[key]);
        d[key] = [...d[key], c[key]];
    } else if (key === 'b') {
        b[key].push(a[key]);
        b[key] = [...b[key], a[key]];
        d[key].push(c[key]);
        d[key] = [...d[key], c[key]];
    }
});

If you imagine the compiler expanding out all code bodies to be an individually-rechecked block that iterates over all possible combinations of union values it closes over, then it'd be possible to check this function as expected, but you can also see why that kind of implementation would be thousands of times slower for any nontrivial block.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label May 17, 2019
@ftrimble
Copy link
Author

ftrimble commented May 17, 2019

I'm somewhat skeptical that this needs to be "thousands of times slower". The important part here is that there is no k2 as argued in your example above:

b[k1].push(a[k2]) is a valid call because k1 could be "a" and k2 could be "b"

This is only operation on k1 and it is not possible that k1 can be both a and b simultaneously - it must be one or the other.

Maybe your point is that

there's no mechanism that correlates that both utterings of key must be the exact same value

But i would argue that's a solvable problem - it is inferrable from the source that key has not changed between invocations, which is precisely why i filed this ticket.

It's hard for me to accept that writing the same block of code for every possible value here is the typescript supported way of doing this... especially when you would have to actually write out every line many times given that this cannot be written as a function or the same issue arises.

@RyanCavanaugh
Copy link
Member

It's hard for me to accept that writing the same block of code for every possible value here is the typescript supported way of doing this

It's not, it was just a demonstration of the fact that the checker can reason about the code in the case where the type is known to be exactly one value.

But i would argue that's a solvable problem

Certainly possible, yes, just a massively complex undertaking we're not inclined to tackle at this point.

@fatcerberus
Copy link

fatcerberus commented May 21, 2019

In other words, the compiler sees at each line “object indexed with key of type 'a' | 'b'” and that’s all the data it has—it doesn’t even have enough information to be able to say “oh, it’s the same variable as last time” (this information simply isn’t tracked). It’s true that TypeScript can narrow types based on conditional statements, but that’s done by a completely a separate mechanism if my understanding is correct.

@fatcerberus
Copy link

fatcerberus commented May 21, 2019

Thinking about this some more, it’s kind of interesting. I’m not convinced it would be enough just for TS to know key is the same value each time; the concrete problem here is that key is type "a" | "b" and therefore we don’t actually know at compile-time what type the indexing operation is going to return.. So TS would need to, besides realizing that key is always the same value, also be able to do a type relation between two unknown types—which to me sounds very challenging.

As human beings, we know if the key is the same, so will the type retrieved, regardless of what types are involved, but this doesn’t automatically follow from the compiler’s perspective unless the compiler is explicitly programmed to track it.

@theRobinator
Copy link

Ran into this today, but with a simpler reproduction that uses only one type (playground link):

interface Foo {
    one: number;
    two: string;
};

function copy(source: Foo): void {
    const newFoo: Foo = { one: 0, two: '' };

    for (const key of ['one', 'two'] as const) {
        newFoo[key] = source[key];  // Error: Type 'string | number' is not assignable to type 'never'.
    }
}

This case might be simpler to solve than the OP because it's indexing into the same type each time. It's also probably much more common than the OP for the same reason.

@hiukky
Copy link

hiukky commented Jan 24, 2020

I really hated that mistake today lol

@ccorcos
Copy link

ccorcos commented Jul 21, 2020

I'm having the same issue I think. Here's what I'm working with:

// Imagine these are records in a database.
type X = { id: string, x: number }
type Y = { id: string, y: number }

// A mapping of which records live in which tables.
type TableToValue = { x: X, y: Y }

type Table = keyof TableToValue
type Value = TableToValue[Table]

// A datastructure representing a bunch of records indexed by table and id.
type ValueMap = {[T in Table]: {[id: string]: TableToValue[T]}}

function getValue<T extends keyof ValueMap>(valueMap: ValueMap, table: T, id: string): ValueMap[T][string] {
  const tableMap = valueMap[table] // Inferred as ValueMap[T] ✅
  const value = tableMap[id] // Inferred as X | Y 🤔
  return value // Broken in v3.9.2, works in v3.3.3 ❌
}

const value = getValue({} as ValueMap, "x", "1")

Playground

@ccorcos
Copy link

ccorcos commented Aug 31, 2020

For others who are running into this issue, I think I've finally figured it out. I'm going leave an explanation here in the interest of others 😄

Let's break it down into a really simple example.

type Key = "x" | "y"
type IdentityMap = {[T in Key]: T}

function getValue<T extends Key>(map: IdentityMap, key: T): T {
  return map[key] // ❌ IdentityMap[T] is not assignable to type T
}

It's frustrating that this does not work because we can observe that it should be correct. And in fact, if we represent the type this way, it does work:

function getValue<T extends Key>(map: IdentityMap, key: T): IdentityMap[T] {
  return map[key]
}

const x = getValue({x: "x", y: "y"}, "x") // Type is "x"

Now let's consider what happens with union types.

const k = "y" as Key
const y = getValue({x: "x", y: "y"}, k) // Type is Key

This makes sense, but now let's try a more complex example:

type Obj<T extends Key> = {key: T}
type ObjMap = {[T in Key]: Obj<T>}

function getObjValue<T extends Key>(map: ObjMap, key: T): Obj<T> {
  return map[key] // ❌ 'Obj<"x"> | Obj<"y">' is not assignable to type 'Obj<T>'
}

This code used to work for me in TypeScript 3.4. Now the code must be written as:

function getObjValue<T extends Key>(map: ObjMap, key: T): ObjMap[T] {
  return map[key]
}

This appears to work the exact same:

const obj = getObjValue2({x: {key: "x"}, y: {key: "y"}}, "y") // Obj<"y">

But the main difference is how TypeScript treats unions. If we pass a key which is a union, we get a union return type.

const k = "y" as Key
const obj = getObjValue2({x: {key: "x"}, y: {key: "y"}}, k) // Obj<"x"> | Obj<"y"> instead of Obj<"x" | "y">

In the old implementation, we would get back Obj<"x" | "y"> which is incorrect and ironically a source of other issues I've had in the past.

@karlhorky
Copy link
Contributor

karlhorky commented Jun 2, 2021

I guess this was also closed due to the "Design Limitation" label? Eg. due to it being a limitation in the design of TypeScript - and this behavior will not be changed?

@jcalz
Copy link
Contributor

jcalz commented Jul 2, 2021

I think #32693 is the still-open issue about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

8 participants