Consider inferring class members types by implemented interface members (continuation of #340)

[dtinth]

Continuation of #340, #1373, #5749, #6118, #10570, #16944, #23911.

Why a new issue?

Many previous issues discussed about having contextual types based on both extended base class members and implemented interface members. This proposal only applies to implements, not extends.

Discussions in #6118 ends with an edge case when a class extends a base class and implements another interface. I think this problem would not occur if the scope is limited to implements keyword only, and I believe would be a step forward from having parameters inferred as any.

Previous issues have been locked, making it impossible to continue the discussion as time passes.

Proposal

Using terminology “option 1”, “option 2”, “option 3” referring to this comment: https://github.com/microsoft/TypeScript/issues/10570#issuecomment-296860943

• For members from extends keep option 1.
• For members from implements:
  • Use option 3 for non-function properties that don’t have type annotation.
  • Use option 2 for function properties and methods.

Examples

Code example in the linked comment:

    class C extends Base.Base implements Contract {
        item = createSubitem(); // ⬅️ No type annotation -- inferred as `Subitem`
    }

Since only the implements keyword is considered, item will be inferred as Subitem.

Example in https://github.com/microsoft/TypeScript/issues/10570#issuecomment-296860943

interface I {
  kind: 'widget' | 'gadget';
}

class C implements I {
  kind = 'widget'; // ⬅️ No type annotation -- inferred as 'widget' | 'gadget'
}

// Above behavior is consistent with:
const c: I = {
  kind: 'widget' // ⬅️ c.kind is also 'widget' | 'gadget'
}

interface I {
  kind: 'widget' | 'gadget';
}

class C implements I {
  kind: 'widget' = 'widget'; // ⬅️ Explicit type annotation required to pin as 'widget'
}

Example in #16944:

interface IComponentLifecycle<P> {
  componentWillReceiveProps?(nextProps: Readonly<P>, nextContext: any): void;
}

interface IProps {
  hello: string;
}

class X implements IComponentLifecycle<IProps> {
  componentWillReceiveProps(nextProps) {
    //                      ^ Contextually typed as Readonly<IProps>
  }
}

Example in #340:

interface SomeInterface1 {
    getThing(x: string): Element;
}

interface SomeInterface2 {
    getThing(x: number): HTMLElement;   
}

declare class SomeClass implements SomeInterface1, SomeInterface2 {
    getThing(x) {
        //   ^ Contextually inferred from
        //     (SomeInterface1 & SomeInterface2)['getThing']
        //     As of TS 3.5, it is an implicit any.
    }
}

// Above behavior is consistent with:
const c: SomeInterface1 & SomeInterface2 = {
    getThing(x) {
        //   ^ Implicit any, as of TS 3.5
    },
}

Checklist

My suggestion meets these guidelines:

• [x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
• [x] This wouldn't change the runtime behavior of existing JavaScript code
• [x] This could be implemented without emitting different JS based on the types of the expressions
• [x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
• [x] This feature would agree with the rest of TypeScript's Design Goals.

[tadhgmister]

I would worry that changing the rules for type inference would introduce cases where types would very subtlety change when only changing base classes which feels not ideal. #36165 suggests a way to just explicitly refer to the type expected in cases like this.

[dtinth]

@tadhgmister It is a very valid concern, which is why this proposal does not apply to base classes; only interfaces.

[tadhgmister]

I don't think that changes my concern, consider the following code:

interface SomeInterface {
  field: "A" | "B"
}
class A { // implements SomeInterface {
  field = "A";
}

let x = new A();
x.field = "C";

Right now this code is perfectly valid, if we add implements SomeInterface to A we currently get an error at A.field. we then (hopefully with some shorthand) change it to be the same type as SomeInterface then get a second error where it is being set to a now invalid value.

This flow forces us to at least acknowledge that A.field is changing types and possibly make a different decision than simply changing it from string to "A" | "B" as needed. This step of looking at A.field during the change seems like a necessity to me.

However with your proposal that intermediate step would be skipped, we add an extra implementation in A and the only error we'd get is somewhere down an inheritance line. We'd be asking the question ""why is assigning to "C" no longer valid? What should I be using instead?"" instead of asking the possibly much more relevant question ""does it still make sense for A.field to be any string or does it need to change types to conform to the new interface? What migration is needed to account for current uses of A.field and still conform to the new interface?""

In most cases where adding a new implemented interface any changes to fields requires some sort of deprecation comments about how to change existing code to account for the new type and if the type is changing automatically without getting a programmer to ever look at the definition then I worry we would miss out on that addition.

[svicalifornia]

@tadhgmister In your example above, you write, "This flow forces us to at least acknowledge that A.field is changing types…"

But that's exactly what is already implied when you add implements SomeInterface. If some class has a property field with any type that is not a subset of "A" | "B", then it does not legally implement SomeInterface. If some function takes a parameter of type SomeInterface, you can't pass any object where field is set to anything other than A or B, because those are the only legal values for field in SomeInterface.

There's no need — and lots of inconvenience — to force developers to re-confirm their intent by explicitly typing field after adding SomeInterface. The field property of class A should automatically inherit type "A" | "B" from SomeInterface, and the developer should have the option of explictly declaring a more specific type if desired, but this must be a subset of "A" | "B".

[svicalifornia]

@dtinth I'm curious to understand your rationale for properties inherited via extends to have a different type inference behavior than those declared via implements. If the intent was to avoid the breaking change given by @RyanCavanaugh in the example code under option 3 of #10570:

class Animal {
    parent = new Animal();
    move() { }
}
class Dog extends Animal {
    parent = new Dog();
    woof() {
        this.parent.woof();
    }
}

That code given is at best vague and—to me, at least—violates the principle of least surprise, especially for developers coming from other classical-OOP languages such as Java. Without an explicit type of parent in class Dog, I would have expected parent to inherit the type Animal from its overridden property of that superclass. That would preserve the greatest compatibility when passing a Dog instance to some function that takes an Animal, and if the developer should need to break that assumed compatibility with a more specific type for parent in the Dog class, then that should be explicitly typed as parent: Dog = new Dog().

I would propose that type inference for any inherited property (via extends or implements) be made as follows:

A. Start by inheriting type from the ancestor classes:

• If the type of the property/method name was explicitly defined in an ancestor class, then use the type from the nearest ancestor class where the type was explicitly defined. (This is similar to option 3 of #10570, except more specific about ancestor selection.)
• If the property/method name was assigned in one or more ancestor classes but always without an explicit type, then use the inferred type from the highest (farthest) ancestor class where the property/method name was used. Without explicit typing, we should expect the inferred type from the highest ancestor class to carry down to each subclass. (This is similar to option 2 of #10570, except more specific about ancestor selection.)
• If the property/method name was not defined or assigned in any superclass, then the ancestor classes do not dictate any type inference, so we start with type unknown.

B. Then restrict the above type by taking its intersection with the same property/method name of all of the interfaces listed in the implements clause of the class. It may turn out that the intersection becomes never, in which case TypeScript should complain that the given interfaces are not compatible with each other or with the types dictated by our ancestor classes. Otherwise we'll get some subset of all the intersected types.

C. If both the ancestor classes and the interfaces have nothing to say about this property/method name, then its type is still unknown at this point. In this case, then we can infer its type from the expression given for its initial value. Property/method types should only be inferred from their initial values when the ancestor classes and implemented interfaces have not already established those types.

That's my proposal. It ensures the greatest polymorphism by default, and I suggest the least amount of surprise.

Yes, it's a breaking change, but I argue that TypeScript's current handling of the above code was already problematic, and letting that get in the way of simpler and better type inheritance is a mistake that has persisted too long.

It seems to me that the TypeScript 4.0 release offers a good opportunity to introduce such a breaking change to set things right.

[dtinth]

I'm curious to understand your rationale for properties inherited via extends to have a different type inference behavior than those declared via implements.

@svicalifornia I try to minimize breaking changes in my proposal in order to make improvements more iterative, so that if the breaking change is deemed unacceptable, at least we can have some inference (for interfaces in this case), and that would be some step forward.

I’m not opposed to adding breaking changes as your proposal said at all, and I agree that TS 4.0 offers a good opportunity for such changes.

p.s. maybe your proposal should be a separate issue?

[tadhgmister]

• If the property/method name was assigned in one or more ancestor classes but always without an explicit type, then use the inferred type from the highest (farthest) ancestor class where the property/method name was used. Without explicit typing, we should expect the inferred type from the highest ancestor class to carry down to each subclass. (This is similar to option 2 of #10570, except more specific about ancestor selection.)

This would absolutely not work. The highest ancestor isn't always assignable to the lowest one:

class Animal {
    parent = new Animal();
    move() { }
}
class Dog extends Animal {
    parent = new Dog();
    woof() {
        this.parent.woof();
    }
}
class Greyhound extends Dog {
    // are you suggesting we infer this as Animal?
    //  that isn't assignable to Dog.parent so this would just be an error, how is that useful?
    parent = new Dog()
}

There's no need — and lots of inconvenience — to force developers to re-confirm their intent by explicitly typing field after adding interface

that is why I'm pushing for #36165, putting field: inherit = ... isn't all that much inconvenience. And by having to put it there you are prompted to consider if it should be something different instead.

[svicalifornia]

• If the property/method name was assigned in one or more ancestor classes but always without an explicit type, then use the inferred type from the highest (farthest) ancestor class where the property/method name was used. Without explicit typing, we should expect the inferred type from the highest ancestor class to carry down to each subclass. (This is similar to option 2 of #10570, except more specific about ancestor selection.)

This would absolutely not work. The highest ancestor isn't always assignable to the lowest one:

class Animal {
    parent = new Animal();
    move() { }
}
class Dog extends Animal {
    parent = new Dog();
    woof() {
        this.parent.woof();
    }
}
class Greyhound extends Dog {
    // are you suggesting we infer this as Animal?
    //  that isn't assignable to Dog.parent so this would just be an error, how is that useful?
    parent = new Dog()
}

I'm saying that if an instance property in a subclass needs a different type than in its superclass, then that new type should be explicitly defined, like this:

class Dog extends Animal {
    parent: Dog = new Dog();
    woof() {
        this.parent.woof();
    }
}

Otherwise, it should inherit the type from its superclass, which maximizes polymorphism by default — in this case, making Dog more interchangeable with its superclass Animal and other subclasses of Animal.

There's no need — and lots of inconvenience — to force developers to re-confirm their intent by explicitly typing field after adding interface

that is why I'm pushing for #36165, putting field: inherit = ... isn't all that much inconvenience. And by having to put it there you are prompted to consider if it should be something different instead.

But we shouldn't need to add an inherit keyword just to get the benefits of type inheritance that other OOP languages provide by default. If we favor type inheritance and require developers to apply more specific types when they are specifically needed, then we will make TypeScript's type inheritance more useful and similar to other languages, and I believe that the preferable choice in the vast majority of use cases.

[lukeed]

I was surprised to find that this wasn't already possible, especially given the fact that plain objects can be quickly stamped out using a complex type without needing to redefine all type/method arguments repeatedly:

type Hello = {
    add(x: number, y: number): number;
}

let demo: Hello = {
    add(x, y) {
        return x + y;
    }
}

This allows the definition/contract to be written once & all implementations must then abide by it.

However, with classes, all classes have to abide by the definition (of course), but each class definition has to redeclare the interface definition. This is unnecessarily duplicative, especially since the implemented interface is already "loaded" and used as part of the type checking:

interface Hello {
    add(x: number, y: number): number;
}

class Demo implements Hello {
    add(x: string, y: string) {
        return x + y;
    }
    //=> "Type '(x: string, y: string) => string' is not assignable to type '(x: number, y: number) => number'."
}

AKA – TS already knows exactly what it's supposed to be.

Requiring that the definition be inlined into every implementation means that the interface is really just an existence and a "repeat after me" check. The let demo: Hello approach allows for strictness and brevity, but classes achieve strictness thru duplication.

[trusktr]

@JSMonk @MaxGraey Let's avoid this in Hegel. Here's a simple example:

class Bar<Foo extends object> {
  m!: Foo
  method(arg: Foo) {}
}

class Test extends Bar<{n: number}> {
  override method(arg) {} // Error, arg has type any, but shouldn't arg have type {n:number} ?
}

https://tsplay.dev/mqQJJm

Here's a bigger example showing the base class implementation details leaking to the subclass author, requiring duplication:

type Options<T, O> = {
  model: ModelBase<T>
  n: number
} & O

type ModelBase<T> = {
  s: string
} & T

class Bar<T extends object, O extends object> {
  m!: ModelBase<T>
  method(arg: Options<T, O>): ModelBase<T> {return {} as any}
}

// User experiences an error
class Test1 extends Bar<{b: boolean}, {s: symbol}> {
  override method(arg) {return {} as any} // arg should have the expected type based on the template (Options<{b: boolean}, {s: symbol}>)
}

// Now the user has to duplicate
class Test2 extends Bar<{b: boolean}, {s: symbol}> {
  override method(arg: Options<{b: boolean}, {s: symbol}>) {return {} as any}
}

// Or consolidate, but it is redundant
interface MyModel {b: boolean}
interface MyOptions {b: boolean}
class Test3 extends Bar<MyModel, MyOptions> {
  override method(arg: Options<MyModel, MyOptions>) {return {} as any}
}

https://tsplay.dev/w24bbm

This is not ideal because now the end user has to know how to apply template types in the same way as the base class, despite that the subclass method is already constrained to the base class method type to begin with.

[matthew-dean]

@lukeed

I was surprised to find that this wasn't already possible, especially given the fact that plain objects can be quickly stamped out using a complex type without needing to redefine all type/method arguments repeatedly:

Not only that, but if you abandon class syntactic sugar (yes, not exactly syntactic sugar, but close), then you get much better TypeScript support. Classes turn out to the least supported primitive in TypeScript. For example, if you use this classic approach...

export interface Print {
  prototype: {
    print(txt: string): void;
  }
}

/** @class */
export const TextBook: Print = function() {}
TextBook.prototype.print = function(txt) {}

Then in the above example, the txt value understands that it is a string. However, there's currently no way to do this (defining a single interface) using class syntax. It simply isn't possible, which is just weird.

[matthew-dean]

Note also that there's no way to externally type the constructor in a single declaration. The following works for all methods, but for the constructor, you need to individually type params.