Skip to content

Incorrect type inference #31741

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
uqee opened this issue Jun 3, 2019 · 6 comments
Closed

Incorrect type inference #31741

uqee opened this issue Jun 3, 2019 · 6 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@uqee
Copy link

uqee commented Jun 3, 2019

TypeScript Version: 3.3.3333

Search Terms:
typescript type guard else never

Code

    enum Type { N, S }

    interface Interface<E extends Type | unknown = unknown> {
      type?: Type
      data?: // just for an idea of how the E parameter is used
        E extends Type.N ? number :
        E extends Type.S ? string :
        number | string
    }

    const isN = (i: Interface): i is Interface<Type.N> => i.type === Type.N

    const i: Interface<unknown> = {}

    if (isN(i)) {
      // i is Interface<Type.N>
    } else {
      // i is never
      // shouldn't it stay Interface<unknown>?
      i
    }

Related Issues:
#17789
#10934

@jack-williams
Copy link
Collaborator

The value v is narrowed on assignment, so while it is declared has having type Type, it actually has type number. From that it follows that isN(v) is always true and never false, hence the narrowed type you see.

If you adjust to declaration of v to prevent narrowing you get the expected behaviour.

const v: Type = 42 as Type;

if (isN(v)) {
    const n: number = v;
} else {
    const s: string = v;
}

@nmain
Copy link

nmain commented Jun 3, 2019

Note that the complex types here are just a distraction; it reproduces fine with const v: string | number = 42

@uqee
Copy link
Author

uqee commented Jun 3, 2019

Thanks for the quick response, guys! Unfortunately, I tried to create a minimal example based on a much more complex code, where the issue has originated, I guess it became too simple to reproduce ) I've updated the example with a little bit more complexity.

@jack-williams
Copy link
Collaborator

jack-williams commented Jun 3, 2019

Ah ok. Yes I think something is not right here. The checker thinks that Interface<unknown> is a subtype of Interface<Type.N> which is why it's getting filtered from the type in the else clause.

const i: Interface<unknown> = { data: "hello" };
let j: Interface<Type.N> = { data: 3 }
j.data = i.data; // error
j = i; // no error

This is likely related to #31295 because you get the correct behaviour if you use two identical but differently named aliases.

enum Type { N, S }

interface Interface<E extends Type | unknown = unknown> {
  type?: Type
  data?: // just for an idea of how the E parameter is used
  E extends Type.N ? number :
  E extends Type.S ? string :
  number | string
}

interface Interface2<E extends Type | unknown = unknown> {
  type?: Type
  data?: // just for an idea of how the E parameter is used
  E extends Type.N ? number :
  E extends Type.S ? string :
  number | string
}

const isN = (i: Interface): i is Interface2<Type.N> => i.type === Type.N;
const i: Interface<unknown> = { data: "hello" };
let j: Interface2<Type.N> = { data: 3 }
j.data = i.data; // error
j = i; // error

if (isN(i)) {
  i // i is Interface2<Type.N>
} else {
  i // i is Interface<unknown>
}

@ahejlsberg
Copy link
Member

This is an effect of the type checker deciding that Interface<E> bi-variant in E (because of the way E is witnessed only in the test of a conditional type). We measure variance in order to avoid repeated structural type checks, but there are known limitations around non-linear types such as the conditional type in the example. So, this is basically a design limitation.

You could consider including an optional property that witnesses E in the interface:

interface Interface<E = unknown> {
    // ...
    __E__?: E;
}

This causes the type checker to measure the interface as co-variant in E and you then get the expected behavior.

@ahejlsberg ahejlsberg added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jun 3, 2019
@uqee
Copy link
Author

uqee commented Jun 4, 2019

Awesome, @ahejlsberg, thanks for your help!

@uqee uqee closed this as completed Jun 4, 2019
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

4 participants