Skip to content

Change in generic behavior between 3.0.1 and >=3.1.1 #27895

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
chrisbouchard opened this issue Oct 14, 2018 · 3 comments · Fixed by #30769
Closed

Change in generic behavior between 3.0.1 and >=3.1.1 #27895

chrisbouchard opened this issue Oct 14, 2018 · 3 comments · Fixed by #30769
Assignees
Labels
Bug A bug in TypeScript Domain: Indexed Access Types The issue relates to accessing subtypes via index access Fixed A PR has been merged for this issue

Comments

@chrisbouchard
Copy link

chrisbouchard commented Oct 14, 2018

TypeScript Version: 3.2.0-dev.20181011
This code worked in 3.0.1, and is broken >=3.1.1.

Search Terms:
"is not assignable to type"

Code

export interface Entity {
    id: number | string;
}

export type IdOf<E extends Entity> = E['id'];

export interface EntityState<E extends Entity> {
    ids: IdOf<E>[];
    entities: { [key: string]: E, [key: number]: E };
}


export function getAllEntities<E extends Entity>(state: EntityState<E>): E[] {
    const { ids, entities } = state;
    return ids.map(id => entities[id]);
}

export function getEntity<E extends Entity>(id: IdOf<E>, state: EntityState<E>): E | undefined {
    const { ids, entities } = state;

    if (!ids.includes(id)) {
        return undefined;
    }

    return entities[id];
}

For completeness, I'll just mention that I intend for users to create subinterfaces of Entity and EntityState for their domain types (like @ngrx/entity), overriding id to be a more restrictive type. This is why the IdOf type is important. E.g.,

interface Task extends Entity {
    id: string;
    due: Date;
    title: string;
}

interface TaskState extends EntityState<Task> {
    // Inherits EntityState#ids as string[]
    currentTask?: string;
}

Expected behavior:
The code compiles without error.

Actual behavior:

BUILD ERROR
projects/entity/src/lib/entity-state.utils.ts(13,5): error TS2322: Type '{ [key: string]: E; [key: number]: E; }[E["id"]][]' is not assignable to type 'E[]'.
  Type '{ [key: string]: E; [key: number]: E; }[E["id"]]' is not assignable to type 'E'.
    Type 'Entity' is not assignable to type 'E'.
projects/entity/src/lib/entity-state.utils.ts(23,5): error TS2322: Type '{ [key: string]: E; [key: number]: E; }[E["id"]]' is not assignable to type 'E | undefined'.
  Type 'Entity' is not assignable to type 'E'.
    Type '{ [key: string]: E; [key: number]: E; }[E["id"]]' is not assignable to type 'E'.
      Type 'Entity' is not assignable to type 'E'.

Playground Link:
Playground Link

Related Issues:
I did not find any that seemed similar, but I'm also unsure what the actual underlying problem is. I did ask on StackOverflow, and they suggested I ask here.

@mattmccutchen
Copy link
Contributor

This was caused by #26698. The old behavior to check assignability of the indexed access type { [key: string]: E; [key: number]: E; }[E["id"]] to E was to take the base constraint of both the object type and the index type, which gave { [key: string]: E; [key: number]: E; }[string | number] = E, which worked. The new behavior takes the constraint of the object type, and if there isn't one, jumps straight to the base constraint of the whole indexed access type, so it goes to Entity and misses E.

@chrisbouchard
Copy link
Author

Is this considered a regression? Or, if this is the intended new behavior, is there a suggested workaround?

@mattmccutchen
Copy link
Contributor

I'd call it a regression, but it's not my opinion that matters.

A workaround is to upcast id to string | number:

export function getAllEntities<E extends Entity>(state: EntityState<E>): E[] {
    const { ids, entities } = state;
    const ids2: (string | number)[] = ids;
    return ids2.map(id => entities[id]);
}

export function getEntity<E extends Entity>(id: IdOf<E>, state: EntityState<E>): E | undefined {
    const { ids, entities } = state;

    if (!ids.includes(id)) {
        return undefined;
    }

    const id2: string | number = id;
    return entities[id2];
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Indexed Access Types The issue relates to accessing subtypes via index access Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants