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

[bug] Prevent generic type information loss from type narrowing #40436

Closed
millsp opened this issue Sep 8, 2020 · 10 comments
Closed

[bug] Prevent generic type information loss from type narrowing #40436

millsp opened this issue Sep 8, 2020 · 10 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@millsp
Copy link
Contributor

millsp commented Sep 8, 2020

TypeScript Version: 4.0.2

Search Terms: type narrowing, intersection, property, any, type loss, property widening, generic type narrowing, instanceof, typeof

Code

This shows how ts fails to preserve generic types after narrowing the type with instanceof:

class A<T> {
  constructor(public value: T) {}
}

const fn = <T>(value: T) => {
  if (value instanceof A) {
    return value;
  }

  return undefined;
};

const fn = wrap(new A(42));

if (test) {
  type valueType = typeof test.value; // any
}

The inferred result of wrap is T & A<any>. Because of any's (or unknown's) widening nature, there is type information loss (widening to unsafe any on properties).

Expected behavior:

Instead of using & in the inferred result, could we use a conditional type instead? At the moment, I do this by hand:

class A<T> {
  constructor(public value: T) {}
}

const fn = <T>(value: T) => {
  if (value instanceof A) {
    return value as any as Extract<T, A<any>>;
  }

  return undefined;
};

const test = fn(new A(42));

if (test) {
  type valueType = typeof test.value; // number
}

Actual behavior:

TypeScript creates intersections, causing generic property type loss.

Playground Link: https://www.typescriptlang.org/play?jsx=0&ts=4.0.2#code/N4Ag9GIA4E4PYCMA2BTAtgWAFAhAYyQEMBnYkAQQB4AVAPhGG11zzgDtiAXGAVz07gwAFFB7IAlnhAA3Qkh4oAXCGoBKBgF8mILVm2sOnEAHcYhKCAC8IGrSGz5Slesv1GOZuIBmIe3IUg4oaEbHgocD7k6u7MzDAonDwwbDL+KADc2ri6WSDxickgPGwAJiheQSglmR4aNfrsXCCcKE3WpuZCbCjGFEIALABMqqr1Ht6+LVzRuZwAnlAoqY7UC0vW84sRza2cAHQOCungkCFz2ro5WKAQ0HCk4shLxHDynOLs+kSkFLYMuQYuLx+IIRGIkJJlgplGpNBdsA1DCYzBZrLY-I4YS43LkJhiAkEuCEwtsov8PLF8kkUocliQQGcGWQAKIAD24hH4NAANL8zrRaDVYldKQlqUVSuVKtULmMWI0jFMjO0UV0en0hiM5YEfEIlTMKc01lCUKtFlYjVsfEqDmljrc2Dw0AgUDB4VhdEA

Related Issues:
Related from far #37993

@millsp
Copy link
Contributor Author

millsp commented Sep 8, 2020

Here's a more complex example to show how the inference could work. I'm "emulating" the behavior I would like so I'm using returnValue to illustrate how the types can be narrowed down with Extract and Exclude:

class A<T = unknown> {
  constructor(public value: T) {}
}

class B<T = unknown> {
  constructor(public value: T) {}
}

const wrap = <R>(fn: () => R) => {
  const returnValue0 = fn();

  if (returnValue0 instanceof A) {
    return returnValue0 as Extract<typeof returnValue0, A>;
    // type-safe narrowing with `A`
  }

  // since `A` was narrowed out, we exclude it out
  const returnValue1 = returnValue0 as Exclude<typeof returnValue0, A>;

  if (returnValue1 instanceof B) {
    return returnValue1 as Extract<typeof returnValue1, B>;
    // type-safe narrowing with `B`
  }

  // since `B` was narrowed out, we exclude it out
  const returnValue2 = returnValue1 as Exclude<typeof returnValue1, B>;

  // at this stage we know for sure that the type of `returnValue` is
  // `A` & `B` excluded: `Exclude<Exclude<R, A<unknown>>, B<unknown>>`

  return undefined;
};

https://www.typescriptlang.org/play?jsx=0&ts=4.0.2#code/MYGwhgzhAECCA8AVaBeaBXAdga0wewHdMA+aAbwFgAoaaYPTCAFwCd1gm8WAKAB3QBGIAJbBoANzAh0AUwBc0RAEpyAX2rqq1UJBgAhJKgw58RUpRp0GzNhy59BIsZOnzFKsps3brTaARYwXiN4ACVibgAzTAVuFRRSUPjzalp6Rj8WGSZ0FkwANSlZAAYjaLiAbmpU6GFI6G4snLzC11LhDLBMYBk8etgPGtom3MxoEZaimVLIaABRAA9WMA54JgBPXl76iYKp4oAaOGIqy1oAenPoDa2AWggwSJloTDAWFkIOgHN-YSYAC2gAANYECat5LJdoBAOj1gaD-LNXu9CDIACbQPDoJhHAjPGQLUDoNHPP6Y7E1dLMcbZUatWQARiMu3p02gs0WRJJa022xpzT2bSOsBO1UsdQaLKmTI6zC6PT60D0gzO-NGasmriZHKWgVWNz5Uq1Rz0otVUIN90ez2RHwI31+AOBejBlghFyuMO6zyBLsRMFtqIxWJx-nxhOkJNqfhDlN8GsFsgATMzaZrGeyYJzIzIeVtFUbGSazTUoWA-ADhDA5V9nnjoLhCNBIlxoblngDy9d-h3eZj6kDCzIgbUIKWriCRwAyZ0jglc9EKIHZ4m5lfc0LC+BYRtmYgm7cmQgkYiumq7YwkyIddGnVQVIA

@millsp
Copy link
Contributor Author

millsp commented Sep 8, 2020

While this could provide better type safety, it can become hard to read it. It would be nice for this to have subtraction types. So instead of having Exclude<Exclude<R, A<unknown>>, B<unknown>>, we would just have R -A -B.

@millsp millsp changed the title Prevent generic type information loss from type narrowing [bug] Prevent generic type information loss from type narrowing Sep 8, 2020
@millsp
Copy link
Contributor Author

millsp commented Sep 8, 2020

I wrote a custom type-guard for instanceOf that has proper inference:

interface Class<P extends any[], O> extends Function { new (...args: P): O; }

type GetClassInstance<C extends Class<any[], any>> =
  C extends Class<any[], infer O> ? O : never;

const instanceOf = <A, C extends Class<unknown[], object>>(
  thing: A,
  Class: C,
): thing is A & GetClassInstance<C> => {
  return thing instanceof Class;
};
class A<T = unknown> {
  constructor(public value: T) {}
}

const fn = <T>(value: T) => {
  if (instanceOf(value, A)) {
    return value;
  }

  return undefined;
};

const test = fn(new A(42));

if (test) {
  type valueType = typeof test.value; // number
}

test.value is now of type number instead of any.

However, it is still not what I would like to achieve, the previous example is not passing.

https://www.typescriptlang.org/play?jsx=0&ts=4.0.2#code/JYOwLgpgTgZghgYwgAgMIBs4GcsB4AKyEAHpCACZbJwgCeA2gLoA0yA8gHxGkQVUBiAVxAIwwAPYhkAb2QgIAd2QAKAHTq4UAOZYAXMnwBKfWwDcyAL4BYAFC2wtAA4oA4hDAZsWAJIgsYGiRcVG4ySjRMHFwaBhZqOg4uAF5bZDRQ3nDPKJimVlAYaHYuAH52ZH15ADdoU1tbBEl-ZFB-QIg2GGQk5FwAQVYQkjCqbLxhAGsQcQUQPORxACMAKwhRROVU5DAAC1AtfQGtsf1UZltjbb2QLRaqPuQAMmQ3D0ifPwCRCGDkrmktlB3IIoFJdvsWp92uIumM6jYLPCEO9kH1cAAVbrISbTWb-LaNT5QQSicRQZSOQSLdDABDIKpwdCCCD6dGGGTWBH1GyE5oKKBwRxYjEcZQMpks5Bs7r4mxpYBdZStL5ITpixnM1h9QzsgFytLIIFgEFScXM+FpTmA4Gg7EUCAwUAQcjwxHc3lgbYQZo9fmC5TyJR9ZQAFgATDr4bYFSpIP5dVsHM56RqIOinCgekmIDCvf5VGaIOYAPTFuSCAC2i2gtgsQA

@millsp
Copy link
Contributor Author

millsp commented Sep 8, 2020

I got a step closer to the desired result with. It fully preserves type information.

type Class<
  P extends readonly unknown[] = readonly unknown[],
  R = unknown
> = {
  new (...args: P): R;
};

type InstanceOf<C extends Class> =
  C extends Class<readonly unknown[], infer O>
  ? O
  : never;
type Select<U, A> =
  U extends A
  ? U
  : never;

type Narrow<A, C extends Class> =
  any extends A
  ? Select<InstanceOf<C>, A>
  : unknown extends A
    ? A & InstanceOf<C>
    : Select<A, InstanceOf<C>>;

const instanceOf = <A, C extends Class>(
  thing: A,
  Class: C,
  ): thing is Narrow<A, C> => {
    return thing instanceof Class;
};

To improve the developer experience, I believe that we would wise to have subtract types:

const fn = <T>(thing: T) => {
  if (thing instanceof A) return 'A';

  // typeof thing = T -A
  if (thing instanceof B) return 'B';

  // typeof thing = T -A -B
  if (thing instanceof C) return 'C';

  return thing; // T -A -B -C
};

This becomes especially interesting if T is a union. That subtracted information can be passed down in the whole program.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 9, 2020
@RyanCavanaugh
Copy link
Member

I'm just going to respond to the OP here because I'm not sure what's going on with the pages of comments that follow.

What you should write is

class A<T> {
  constructor(public value: T) {}
}

const wrap = <T,>(value: T) => {
  if (value instanceof A) {
    return value as A<T>; // <-- type assertion
  }

  return undefined;
};

because you're assuming that any A is an A<T>, even though this isn't true.

@millsp
Copy link
Contributor Author

millsp commented Sep 9, 2020

Thanks @RyanCavanaugh for your fast reply

class A<T> {
    constructor(public value: T) {}
  }

const fn = <T,>(value: T) => {
    if (value instanceof A) {
        return value;
    }

    return undefined;
};

const a = new A(42);

const test = fn(a);

We do know for sure that a is an instance of A, why intersect it with A<any>? Wouldn't it make sense to intersect with A<unknown>? Maybe I'm missing some edge case, is there a reason for this?

type testType0 = (A<number> & A<unknown>)['value']; // number
type testType1 = (A<number> & A<any>)['value']; // any

@millsp
Copy link
Contributor Author

millsp commented Sep 10, 2020

because you're assuming that any A is an A<T>, even though this isn't true.

I don't understand why this is not the case. If A<number> is A<any>, why can't we safely say that the return value is A<number>?

@jcalz
Copy link
Contributor

jcalz commented Sep 11, 2020

@RyanCavanaugh I think return value as A<T> is not the intent, since that would make wrap(new A("hello")) return an A<A<string>> instead of A<string>.

As far as I can tell this issue is a duplicate of #4742 or #28560, with the comments in #30161 being particularly relevant. If you have a value x of type X, and are checking x instanceof C where the instance side of C is generic like C<T>, right now that narrows x to X & C<any> which, while true enough, is often not useful. You really want something like the union of all possible specifications of C<T> that overlap with X.

@millsp
Copy link
Contributor Author

millsp commented Sep 11, 2020

Oh sorry, I completely omitted to rename wrap from another example. It does not actually wrap, that might have led to the confusion. Thanks @jcalz for linking all the relevant issues 🙌 We can mark this a duplicate

@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.

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

4 participants