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

Object type unions inside interfaces result in indexed access being too complex #35216

Closed
henry-alakazhang opened this issue Nov 20, 2019 · 8 comments
Assignees
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@henry-alakazhang
Copy link

henry-alakazhang commented Nov 20, 2019

TypeScript Version: 3.7.2 (anything > 3.5)
Search Terms: indexed access type union too complex to represent

Code

// convoluted example, maybe, but this is the simplest code
// feel free to imagine it's fetching an item that may or may not exist
export function explicitMaybeItem(
  item:
    | NestedUnionOne
    | NestedUnionTwo
    | NestedUnionThree
    | NestedUnionFour
    | NestedUnionFive
) {
  return Math.random() ? item : undefined;
}

// in theory Collections[K] should be the same as the explicit union above
// (ie. a union of 50 different items)
// but it seems like TS blows it out out to 10^5 rather than 10*5
export function maybeItem<K extends keyof Collections>(item: Collections[K]) {
  return Math.random() ? item : undefined;
      /* ^ Expression produces a union type that is too complex to represent. */
}

// These NestedUnions are just unions of 10 different types
export interface Collections {
  readonly one: NestedUnionOne;
  readonly two: NestedUnionTwo;
  readonly three: NestedUnionThree;
  readonly four: NestedUnionFour;
  readonly five: NestedUnionFive;
}

Expected behavior:
Compiles

Actual behavior:
Inside maybeItem, the return line errors withExpression produces a union type that is too complex to represent.

Things that do make it compile:

  • String unions instead of object unions
  • Using the same NestedUnionOne as every type in Collections

Things that don't make it compile:

  • Giving NestedUnioneOne, NestedUnionTwo etc all the same definition

It looks like the compiler is exploding the union out to 10^5 instead of 10*5 and "ignoring" it because it's over the 100k limit. Not sure if there's a solution in that case.

For some context, this is for a document store, where Collections represents the types of all the possible documents. Some of the documents are (usually discriminated) union types, such as notifications. The maybeItem example is convoluted but a more realistic example would be getItem<K keyof Collections>(collection: K, id: number) => Collections[K] | undefined.

Playground Link: Playground link

@henry-alakazhang henry-alakazhang changed the title Object type unions inside interfaces results in indexed access being too complex Object type unions inside interfaces result in indexed access being too complex Nov 20, 2019
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Dec 5, 2019
@weswigham
Copy link
Member

// in theory Collections[K] should be the same as the explicit union above

Ah, and that's where you're wrong. It's not the same, it's much stricter. Notably, when assigning to a T[K], you need to be assignable to all possible values of the index, not any (as is the case for a union). This means when you try to assign item to Collections[K], we need to construct the intersection of each possible index, not the union. So that means NestedUnionOne & NestedUnionTwo & NestedUnionThree & NestedUnionFour & NestedUnionFive. If each of those types is a reference to 10 or so members in a union, the resulting union after normalization (where the unions are hoisted from inside the intersection to outside), is massive.

@weswigham weswigham added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Needs Investigation This issue needs a team member to investigate its status. labels Dec 5, 2019
@typescript-bot
Copy link
Collaborator

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

@aboyton
Copy link

aboyton commented Dec 11, 2019

I'm a bit confused, why do you need to be able to write to all of the possible values, not just one value? If I read from Collection[K] I get something of a particular type, not all of them. Why do I need to be able to write to any of them if I read a specific type?

Let's take an example of a somewhat standard Redux store but with some very un-Redux mutable getters and setters as they illustrate the issues better. Let's say I have a store with three things in it.

interface Store {
  counter: number;
  todo: string;
  done: boolean;
};

const store: Store = {
  counter: 123,
  todo: 'get bread',
  done: false, 
}

Then I can define a generic getter and setter for this, and everything seems work fine.

function set<T extends keyof Store>(type: T, value: Store[T]) {
  store[type] = value; // This works as I would have expected
}

function get<T extends keyof Store>(type: T) {
  return store[type]
}

If you look at the return values of get they are what I would have expected.

const counter = get('counter'); // This is of type number
const todo = get('todo');       // This is of type string
const done = get('done');       // This is of type boolean

Similarly calling the setters works as expected, accepting values of the right types and not the wrong ones.

set('counter', 42); // Works
set('done', 1);     // Argument of type '1' is not assignable to parameter of type 'boolean'.ts(2345)
set('done', true);  // Works

Looking at the types, they are what I would have expected.

type Done = Store['done']; // This is `boolean`

The intersection of these types in Store is of course never.

type All = Store['counter'] & Store['todo'] & Store['done']; // never

But it seems like indeed the actual type Store[K] is the intersection and thus never (as you described). You can see this if you try to assign a wrong value inside the get function.

function set<T extends keyof Store>(type: T, value: Store[T]) {
  store[type] = 1; // This returns an error.
}

this gives an error

(parameter) type: T extends "counter" | "todo" | "done"
Type '1' is not assignable to type 'Store[T]'.
  Type '1' is not assignable to type 'never'.ts(2322)

So it seems the only reason store[type] = value; works is that they are both never. This seems less than ideal to me and presumably not what people are expecting.

Interestingly, if inside get I write

let foo: Store[T];
foo = 1;

I get the error Type '1' is not assignable to type 'Store[T]', but if I write:

let bar: Store['counter' | 'todo' | 'done'];
bar = 1;

then this works. Which I think shows I'm misunderstanding something. I had assumed previously that my issues with needing to occasionally cast via any in functions like these was because keyof Store is of type "counter" | "todo" | "done" and there was no way of specifying a generic function would take in a "counter" OR a "todo" but not "counter" | "todo" so you could always write the following:

function test(unknownType: 'counter' | 'todo') {
  const unknownValue = get(unknownType); // type: number | string
}

but it seems like this is not a reason.

Can you explain why it is that you need to be able to assign to all not any? This seems to restrict the ability to nicely type a Redux store. The fact that the get function above works seems more of a bug than a feature to me.

@weswigham
Copy link
Member

Can you explain why it is that you need to be able to assign to all not any?

When you read a value of type T[K], it could be any of the props on T to which any possible K could refine to, however when you write a value to a T[K], the value you write needs to be valid for all possible instantiations of T and K, as any of them could be instatiated to later on. If you only wrote the type of, say "fooProp", but then got called with K="barProp", the type you'd have returned earlier would have been for the wrong value. When you're working with concrete values, the sets of types for "all possible T" and "all possible K" are known, so it's easy to know what to allow; however, when T or K are generic, we have to work with the notion of "some aribitrary subtype of the constraint" (vs a specific set of subtypes), which is much stricter.

@aboyton
Copy link

aboyton commented Dec 11, 2019

Does this mean that code like

function set<T extends keyof Store>(type: T, value: Store[T]) {
  store[type] = value;
}

function get<T extends keyof Store>(type: T) {
  return store[type]
}

is wrong and should not be expected to compile? Is there a better way to be writing code like this?

@weswigham
Copy link
Member

Nope, those are fine (ish) - T is linked between Store[T] and T - it's the same T. Technically, that's a little too permissive, since you could say T = "a" | "b", then provide "a" for type and Store["b"] for value; but we make the assumption that the two instances of T are linked and identical, and so shouldn't have a problem.

The issue in your original example is actually undefined (sortof). When you say expr ? a : b, we evaluate the type of that expression as the union of the types of the two branches. However, to reduce verbosity, we do something called subtype reduction - this takes that union, and removes redundant elements from it. This way, eg, if I write expr ? ("a" as string) : ("b" as "b"), we get string and not string | "b", which is redundant. In the case of Collection[K] | undefined, we want to know if Collection[K] is a subtype of undefined, or vice-versa. To see if it is, we need to know if, for all possible values of K, undefined is a subtype of all the Collection[K]. So we want to know if undefined is a subtype of NestedUnionOne, and NestedUnionTwo, and NestedUnionThree, and so on. That means that undefined is a subtype of Collection[K] if it's a subtype of all of those things at the same time - an intersection. So we check if undefined is a subtype of NestedUnionOne & NestedUnionTwo & ... and so on.

To see how this affects your code in a concrete way, take your example from the OP and add:

type NestedUnionOne = "a" | undefined;
type NestedUnionTwo = "b" | undefined;
type NestedUnionThree = "c" | undefined;
type NestedUnionFour = "d" | undefined;
type NestedUnionFive = "e" | undefined;

(and make sure you're in strict mode). Check the return type of maybeItem - it should be Collections[K]. We've determined that every possible outcomes of the indexing already includes undefined, so there's no need to include it. Now, change NestedUnionFive to be type NestedUnionFive = "e", and look at maybeItem's return type - now it's Collections[K] | undefined, because there are cases for which undefined is not a Collection[K], and so must be separately represented.

@aboyton
Copy link

aboyton commented Jan 16, 2020

Thanks for your really detailed responses. I always appreciate the work you and your team do, fixing issues and taking the time to provide really helpful responses.

The unification to never still seems a bit strange to me, but we've been able to work around the problem by breaking a bunch of huge union types (as well as Collections for the document type we had one for the backend return type, etc.) into LOT of smaller types, one per collection that themselves contain their document type, their backend return type, etc. We can then index these types with CollectionType['Document'] or CollectionType['BackendReturnType'] in each function. This stops this huge state explosion, and removes the huge god-objects that we had so appears to be working well.

We hit a new issue with this approach with unification so I've made a new issue #36245.

@poseidonCore
Copy link

Postscript: This behaviour happens only when strictNullChecks is on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

6 participants