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

Defining a type predicate breaks inheritance for methods with mapped types #56133

Closed
johnw42 opened this issue Oct 16, 2023 · 8 comments Β· Fixed by #56640
Closed

Defining a type predicate breaks inheritance for methods with mapped types #56133

johnw42 opened this issue Oct 16, 2023 · 8 comments Β· Fixed by #56640
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Milestone

Comments

@johnw42
Copy link

johnw42 commented Oct 16, 2023

πŸ”Ž Search Terms

type predicate mapped

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about type system behavior

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.2.2#code/IYIwzgLgTsDGEAJYBthjAgglEBLawUAngDwAqAfAgN4BQCCokM8SwAdgMoAWUu7AawDq+bgHsArhADCY9hACmADwgAKAG7BkEhQC4EEwezEB3dgEp9m7QoS4MZANy16jcAVZKiAL2+kAqhjKiuwAJhiGAsZmANoAuhSqrgxK+tQIMQDSduwIAgpEYgBmCIFx+oFZcQgAvgA0rpZYOPgwxCSBFM41LkweiChoGACyRNh4BO2UCMEKYRjjrYSk03QMfSyIXr4BQSpz4QZGpuzxickIqTQZ2fx5BcWlYOVPVbUNDE2Lk7tdtD1AA

πŸ’» Code

abstract class Arbitrary<T> {
  abstract canShrinkWithoutContext(value: unknown): value is T;

  abstract xyzzy<Us extends unknown[]>(
    x: { [K in keyof Us]: Us[K] },
  ): Arbitrary<Us>;
}

abstract class MyArbitrary<T> extends Arbitrary<T> {
  abstract xyzzy<Us extends unknown[]>(
    x: { [K in keyof Us]: Us[K] },
  ): Arbitrary<Us>;
}

πŸ™ Actual behavior

The compiler reports an error in MyArbitrary unless canShrinkWithoutContext is removed or its return type is changed.

πŸ™‚ Expected behavior

The xyzzy method should be overridden in the derived class.

Additional information about the issue

No response

@Andarist
Copy link
Contributor

It's not the type predicate but rather the reference to T. The same happens with abstract test(): T and similar.

Note that your xyzzy method returns Arbitrary<Us> and that becomes T within Arbitrary. I think that, in a way, you are asking for this to be allowed:

const item: Arbitrary<string> = {} as MyArbitrary<number>

We can see a somewhat better error here as it at least refers to the method that references T but the origin of the problem is truly in the return type of xyzzy

@RyanCavanaugh
Copy link
Member

This seems like a bug:

// No constraint: OK
{
  interface Arbitrary<T> {
    cov: T;
    xyzzy<U>(x: { [K in keyof U]: U[K] }): Arbitrary<U>;
  }

  class MyArbitrary<T> implements Arbitrary<T> {
    cov!: T;
    xyzzy<U>(x: { [K in keyof U]: U[K] }): Arbitrary<U> {
      return null as any;
    }
  }
}

// Use a type alias: OK
{
  type SomeMap<U> = { [K in keyof U]: U[K] };
  interface Arbitrary<T> {
    cov: T;

    xyzzy<U extends unknown[]>(x: SomeMap<U>): Arbitrary<U>;
  }

  class MyArbitrary<T> implements Arbitrary<T> {
    cov!: T;
    xyzzy<U extends unknown[]>(x: SomeMap<U>): Arbitrary<U> {
      return null as any;
    }
  }
}

// Neither: Error
{
  interface Arbitrary<T> {
    cov: T;
    xyzzy<U extends unknown[]>(x: { [K in keyof U]: U[K] }): Arbitrary<U>;
  }

  class MyArbitrary<T> implements Arbitrary<T> {
    cov!: T;
    xyzzy<U extends unknown[]>(x: { [K in keyof U]: U[K] }): Arbitrary<U> {
      return null as any;
    }
  }
}

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Oct 17, 2023
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Oct 17, 2023
@johnw42
Copy link
Author

johnw42 commented Oct 17, 2023

BTW, I noticed last night the weirdness that is { [K in keyof U]: U[K] } being a different type from just U, but it definitely is because it extends object even when U itself doesn't. Not sure if it's related but it sure seems fishy. I found that pattern in fast-check, which seems like a fairly high-profile library. It's in the defintion of a user-facing function in record.ts:

function record<T>(recordModel: { [K in keyof T]: Arbitrary<T[K]> }):
    Arbitrary<RecordValue<{ [K in keyof T]: T[K] }>>;

I think the variable T really should require an extends object constraint, since I don't see how a call to that function could possibly type check otherwise. My guess is that using a trivial mapped type is a workaround someone discovered to avoid introducing that constraint for whatever reason.

@fatcerberus
Copy link

My guess is that using a trivial mapped type is a workaround someone discovered to avoid introducing that constraint for whatever reason.

I don't know if this is the case here but

type ID<T> = { [K in keyof T]: T[K] };

is a common pattern people sometimes write to get TS to fully expand object types in hover tips. AFAIR it's just a no-op on primitive types so it's nice to be able to use it in situations where an object constraint would be inconvenient.

@gabritto
Copy link
Member

I'm looking into this, so far I think the discrepancy in #56133 (comment) is due to inference, which we perform when we compare two signatures that have type parameters (in this case the signatures for xyzzy).

@gabritto
Copy link
Member

Ok, as I mentioned above, the reason why the code errors is, I think, because of inference.

What happens when type checking the code is roughly the following:

  • We check that MyArbitrary<T> is assignable to Arbitrary<T>, because MyArbitrary<T> extends Arbitrary<T>.

    • When doing that, we check if the type of property xyzzy of MyArbitrary<T> is assignable to the type of the same property in Arbitrary<T> .

      • To check if the types of property xyzzy are assignable, we check if source signature <Us extends unknown[]>(x: { [K in keyof Us]: Us[K] }): Arbitrary<Us> (from MyArbitrary<T>) is assignable to target signature <Us extends unknown[]>(x: { [K in keyof Us]: Us[K] }): Arbitrary<Us> (from Arbitrary<T>).

        • Because the source signature has type parameters that are different from the target signature's type parameters, when checking that the source is assignable to the target, we instantiate the source signature in the context of the canonical form of the target signature (i.e. we call instantiateSignatureInContextOf), where the canonical form of the target signature is (x: { [K in keyof Us]: Us[K] }): Arbitrary<Us>.

          • During instantiateSignatureInContextOf, we have source signature <Us extends unknown[]>(x: { [K in keyof Us]: Us[K] }): Arbitrary<Us>, target signature (x: { [K in keyof Us]: Us[K] }): Arbitrary<Us>. This is where type inference kicks in: what instantiateSignatureInContextOf does is essentially to infer type arguments for the source signature's type parameters, and then instantiate the source signature with those inferred type arguments. It infers the source's type arguments by inferring (i.e. call inferTypes) between the source's parameter types and the target's parameter types, and inferring types between the source's return type and the target's return type. This is the part that causes the problem: we infer type argument unknown[] for source's type parameter U, and then we instantiate the source signature with this inference, obtaining a new source signature (x: unknown[]): Arbitrary<unknown[]>.
        • We then check if (x: unknown[]): Arbitrary<unknown[]> is assignable to (x: { [K in keyof Us]: Us[K] }): Arbitrary<Us>, and find that it is not assignable, because the source's return type Arbitrary<unknown[]> is not assignable to the target's return type Arbitrary<Us>, because Arbitrary<T> is covariant on T. This is why the error goes away if we remove abstract canShrinkWithoutContext(value: unknown): value is T inside Arbitrary: Arbitrary becomes independent of T, and then we find that Arbitrary<unknown[]> is assignable to Arbitrary<Us>.

All the variants in this comment that make the error go away do so by changing how we infer the type arguments for the source signature during instantiateSignatureInContextOf, making it so that we infer type argument Us (from the target signature) for type parameter Us in the source signature. We end up checking if (x: { [K in keyof Us]: Us[K] }): Arbitrary<Us> is assignable to (x: { [K in keyof Us]: Us[K] }): Arbitrary<Us> (where both Us are the same type parameter), which is true, so everything works.

@DanielRosenwasser
Copy link
Member

Let's try to use a cleaner example for all discussions here:

declare class Base<T> {
    someProp: T;

    method<U extends unknown[]>(x: { [K in keyof U]: U[K] }): Base<U>;
}

declare class Derived<T> extends Base<T> {
    method<U extends unknown[]>(x: { [K in keyof U]: U[K] }): Base<U>;
}

@gabritto gabritto self-assigned this Nov 23, 2023
@gabritto gabritto removed the Help Wanted You can do this label Nov 23, 2023
@ahejlsberg
Copy link
Member

Let's use unique type parameter names when discussing issues related to unification:

declare class Base<T> {
    someProp: T;
    method<U extends unknown[]>(x: { [K in keyof U]: U[K] }): Base<U>;
}

declare class Derived<S> extends Base<S> {
    method<V extends unknown[]>(x: { [K in keyof V]: V[K] }): Base<V>;
}

Makes it clearer that U and V need to be unified before we can relate the two signatures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants