Abstract classes that implement interfaces shouldn't require method signatures

[laughinghan]

TypeScript Version: 2.7

Search Terms: abstract class implements interface

Code

interface FooFace {
    foo();
}
abstract class FooClass implements FooFace {
    //         ^^^^^^^^
    // Class 'FooClass' incorrectly implements interface 'FooFace'.
    //   Property 'foo' is missing in type 'FooClass'.
    //
    // unless the following is uncommented:
    //abstract foo();
}

// contrast:
abstract class BarClass {
    abstract bar();
}
abstract class BarSubClass extends BarClass {
    // no `abstract bar();` required
}

Expected behavior:
Abstract classes that implement interfaces don't need to define abstract type definitions to satisfy interface.

Actual behavior:
They do.

Playground Link

Related Issues:

• Implement interface helper does not consider what is already implemented on base classes #19847, where typescript fails to see that a superclass does implement a method (contrast this ticket, where typescript fails to see that a class need not implement a method)

[laughinghan]

Apparently this can be worked-around with class and interface merging:

interface FooFace {
    foo();
}
abstract class FooClass implements FooFace {}
interface FooClass extends FooFace {}

let x: FooClass;
x.foo(); // no error

That seems redundant, though, I still think this should be fixed.

#6413 (comment)

[RyanCavanaugh]

If foo is optional in FooFace, then does FooClass have an implicit abstract foo?(); or not?

[laughinghan]

@RyanCavanaugh Yes, definitely. In my real-world use case (in which I'm currently using the interface/class merging workaround), my interface actually only has optional methods, I just thought I'd leave that out of this example for simplicity. abstract Class implements Interface should imply interface Class extends Interface.

[RyanCavanaugh]

This is all intuitive enough for required members but we can't figure out what should happen to optional members. All choices seem bad.

If we copy down optional members and create optional or non-optional abstract members, then there's no way for your derived class to say "I don't want to implement that member".

If we copy down optional members and create optional concrete members, then we're going to copy down optional properties (I'm presupposing here that differentiating properties and methods is right out) and you won't have any way for you to say "I don't want to have that property".

If we don't copy down optional members, then we've created a "hole" in your type that will allow a derived class to accidently overwrite them with a mismatched type:

interface I { x?: number; }
abstract class A implements I { }
// Would not be an error, but could break at runtime
class B extends A { x: string }

The last problem is of course present today, but doing some of the copying would seem to create the appearance that we were doing more than was actually happening.

Overall it seems like we'd be trading off understandability for convenience here.

[shicks]

There actually is a way to copy optional members concretely and have a subclass declare that they don't want to implement it [playground]:

interface I {
  x?: number;
  m?: (x: I) => void;
}
abstract class A implements I {
  x?: number;          // proposal: implicit if no 'x' is specified
  m?: (x: I) => void;  // proposal: implicit if no 'm' is specified
}
class C extends A {
  x?: never;  // "I don't want an x property"
  m?: never;  // "I'm not going to implement this and nobody else better either"
}
class D extends C {
  x?: string;  // error: Type 'string | undefined' is not assignable to type 'undefined'.
  m(x: I) {}   // error: Type '(x: I) => void' is not assignable to type 'undefined'.
}

Today's state is both error-prone and unergonomic, and the only sound thing to do if a class wants to declare it won't implement an optional property is to carry around the tombstone to guarantee nothing implements it in an incompatible way. Making that (presumably rare case) explicit with a "never" seems like the best approach.

[laughinghan]

@RyanCavanaugh Thank you for the thoughtful response, sorry for my delayed reply.

I don't understand why this option seems bad?

If we copy down optional members and create optional concrete members, then we're going to copy down optional properties (I'm presupposing here that differentiating properties and methods is right out) and you won't have any way for you to say "I don't want to have that property".

@shicks is exactly right: the way you say "I don't want to have that property" should be the same as the way you do so when extending the interface, with never:

interface I { x?: number; }
interface J extends I { x: never }

// contrast:
interface K extends I {}
abstract class L implements I {}
class M implements {}

By contrast, you expect K['x'] to be number | undefined. I'm actually completely surprised that L['x'] and M['x'] are invalid rather than also number | undefined.

[shicks]

For what it's worth, I'm interested in this because Closure Compiler (very recently) allows abstract classes to not redeclare unimplemented interface methods, and I don't want to see code taking advantage of this feature be more problematic to migrate to TypeScript.

@RyanCavanaugh what's the process to actually put together a proposal for this? I think the basis is sound, and this should be perfectly viable.

[cassiodsl]

Another simple option, but it is still redundant is:

export interface IonicLifeCycle {

    ionViewWillEnter();
    ionViewDidEnter();
    ionViewWillLeave();
    ionViewDidLeave();
}

and the abstract class :

export abstract class Base implements IonicLifeCycle {

    abstract ionViewWillEnter();
    abstract ionViewDidEnter();
    abstract ionViewWillLeave();
    abstract ionViewDidLeave();
}

[TwoAbove]

I also encountered this issue. This is what I want to do:

interface IQueue<T> {
	run: (job: Agenda.Job<T>, done: (err?: Error) => void) => Promise<void>;
}

abstract class Queue<T> implements IQueue<T> {
	public abstract run; // This will be implemented in classes
}

class Q extends Queue<IJob> {
	async run(job, done) { } // implementation
}

But run in Q has typings of (any, any), when I expect for it to have typings of (job: Agenda.Job<IJob>, done: (err?: Error) => void)

[cassiodsl]

@TwoAbove try this structure:

// Be aware that I have removed the inner class Job just for test purpose.
interface IQueue<T> {
    run(job: Agenda, done: (err?: Error) => void): Promise<void>;
}


abstract class Queue<T> implements IQueue<T> {
    public abstract run(job: Agenda, done: (err?: Error) => void): Promise<void>; // This will be implemented in classes
}


class Q extends Queue<IJob> {
    public async run(job: Agenda, done: (err?: Error) => void): Promise<void> {
        // impl
        return null;
    }
}

// how it looks like, trying to call test.run with no correct properties will give you error
class RunsQ {
    private test: Q;

    constructor() {
        this.test = new Q();
    }

    public myTest(): void {
        this.test.run({name: ''}, null);
    }
}

[syffs]

2 years later...

this seems like an obvious and basic feature for any object-oriented language: how come it hasn't been implemented ?

[iskrid]

Same here, I cannot imagine that there's a problem to solve that, from my p.o.v. it's just a fix in the compiler and won't lead to any issue on runtime.
Any news about it?

[marcus-benojo]

Same, most OO languages have the similar feature

[Elvynia]

Please, would someone update on the state of this issue ? I'm going to use the workaround but I'd really appreciate knowing more about a future proposal !

[jimmykane]

Ok came here to say wtf .... How can this be missing ?

[T-Skinner]

I want this fixed in typescript!

[GiovanniCardamone]

Can't believe this still a problem omg

[xpl]

Please fix it!

[sadra]

Uh, It's going to be three years!

[RyanCavanaugh]

FYI, just dropping "Ugh can't believe it's not fixed already" comments does not change our view of feature priorities. There are outstanding issues with "just doing it" outlined above that haven't been addressed yet. Please engage in meaningful discourse with the content of the thread so far instead of idly complaining.

[sadra]

FYI, just dropping "Ugh can't believe it's not fixed already" comments does not change our view of feature priorities. There are outstanding issues with "just doing it" outlined above that haven't been addressed yet. Please engage in meaningful discourse with the content of the thread so far instead of idly complaining.

So, how can I help to solve this issue?

[RyanCavanaugh]

So, how can I help to solve this issue?

If I knew how to solve it, I'd have done so already 🙃. The desirability isn't the missing piece of this - it's clear that this would be nice - but rather that we lack a solution without negative side effects that outweigh what is ultimately a pure convenience thing.

We'd need some suggestion for how to implement this in a way that isn't starkly surprising or bad in any of the outlined scenarios.

[sadra]

> So, how can I help to solve this issue? If I knew how to solve it, I'd have done so already 🙃. The desirability isn't the missing piece of this - it's clear that this would be nice - but rather that we lack a solution without negative side effects that outweigh what is ultimately a pure convenience thing. We'd need some suggestion for how to implement this in a way that isn't starkly surprising or bad in any of the outlined scenarios.

I'll try to find a solution and ASAP I found it, I'll share it here. Thank you for all those things that you do for this community 🥰🙏

[matigda]

So, how can I help to solve this issue?

If I knew how to solve it, I'd have done so already . The desirability isn't the missing piece of this - it's clear that this would be nice - but rather that we lack a solution without negative side effects that outweigh what is ultimately a pure convenience thing.

We'd need some suggestion for how to implement this in a way that isn't starkly surprising or bad in any of the outlined scenarios.

Can you provide some details about negative impact that you are worrying about?

[sadra]

Can you provide some details about negative impact that you are worrying about?

I have some interfaces that explain the behaviors, and when I implement them in the abstract class, I expect the abstract to inherit those behaviors. But in typescript, I should implement the interface's methods in an abstract class that introduces redundant and unnecessary boilerplate codes.

[matigda]

I understand how it works now, but I wanted to know what holds @RyanCavanaugh from implementing.

[RyanCavanaugh]

This comment outlines the conceptual difficulty of what this actually should mean

[shicks]

I believe I addressed the concerns in that comment pretty thoroughly. The answer is that it should copy all properties, and that if it's intended to not have the property, then the implementer should declare it as optional never, which is a win for consistency, understandability, correctness, and safety all at once.

[RyanCavanaugh]

That's a breaking change, though. This code is currently legal and would become illegal:

interface Something {
    go(): void;
    opt?: number;
}
abstract class M implements Something {
    abstract go(): void;
}

declare const x: M;
const m: { opt?: string, go(): void; } = x;

[shicks]

That's a good point. Unfortunately you're going to break code when you improve safety, by definition. Expanding on your example,

m.opt = 'string';
const s: Something = x;
expectOptionalNumber(s.opt); // whoops

If the assignment to m needs to be legit, then M should have been explicitly declared with opt?: never, and then your example type checks.

That said, it's only a minor safety improvement: you could still assign a string to m.opt and try to read a number out of s.opt, which is a result of the fact that writable properties are properly invariant rather than covariant, so that's a deeper problem that's not really feasible to ever fix. Ultimately, it's not clear which trade-off makes most sense here. If it were being invented today, I'd argue that implicit inheritance is safer and more ergonomic, but since the safety improvement is only partial and it's actually a breaking change, it's possible that the churn to roll it out ends up outweighing the benefit.

I'd be interested in how much actually breaks in the real world, though, and whether any of the breakages actually point to design flaws in the underlying code.

[gkdn]

I'm not sure if it was mentioned before but;
Apart from boilerplate and maintenance burden on abstract class, this behavior also causes unnecessary breaking changes to upstream libraries where only an abstract implementation of the contract exist; requiring them to do new releases whenever parent interface changes.

[shicks]

@gkdn Can you point to a concrete project where this occurred?

I wonder if there's an incremental "half-way" sort of approach, where an initial version could support only non-optional properties in an intuitive way, and then a potential future update could later add support for the optional ones? Is it too confusing/surprising if only required properties are implicitly copied? One should be able to reason about it as copying the bare minimum required to actually satisfy the interface, and optional properties are not required (by definition) so they wouldn't be copied.

Would that still be a breaking change?

[gkdn]

Sorry, maybe I wasn't clear earlier. I was not following the discussion on optional properties but I was only referring one of the limitation of the existing behavior of the type system on abstract classes.
Since I'm not actively developing in TS but only interoping with it, I cannot give you a concrete project where this occurred. However I can illustrate how easily this could happen:

Library 1:

interface IComponent {
  onUpdate():void;
  onRemoval():void;
}

Library 2:

abstract AbstractFancyComponent implements IComponent {
  onUpdate() {
     ....
  }

  // This declaration is necessary today to make the TS type-system happy.
  abstract onRemoval():void;
}

All changes to IComponent#onRemoval (e.g. changing return type) or API additions to IComponent (e.g. add onFoo) are are source-incompatible changes in TS today and requires AbstractFancyComponent to be updated and new release for Library 2 to be pushed.

However Library 2 wouldn't need an update and a re-release if the type system didn't require redeclaration of the abstract methods.

[alimm1995-seequent]

Sorry, I'm not very good at TS, but would it be a good solution to add some kind of new keyword, directive, or new instruction to fix this instead of trying to fit this functionality in the existing implements? For example, I can think of the two following solutions:

1. Instead of implements use a different keyword (e.g. extends) to express the fact that you want the interface to be copied down into the class. For example:

interface FooFace {
    foo();
    bar();
}
abstract class FooClass extends FooFace {
    //                   ^^^^^
    // Instead of "implements" use "extends" to convey that you want all
    //     of the properties in the interfaces to be copied down into the FooClass.
    // I'm sorry if this is a bad suggestion. I don't know what the implications of this could 
    //     be, but we can maybe find a new keyword if using "extends" is a bad idea.

}

2. Instead of having to copy all the properties , one by one, from the interface down into the implementing class, We might be able to just write a single directive to do this for us. For example:

interface FooFace {
    foo();
    bar();
}
abstract class FooClass implements FooFace {
    "use properties"
    //  ^^^^^^^^
    // Write some kind of keyword or directive in the class to say that 
    //     you want the properties to be copied down into the class.
    // Again, I'm sorry if it is a bad suggestion, because I'm not even sure 
    //     if there are any existing directives that can be written in a inner scope
    //     in a file, and I don't know of any other examples in TS that are similar
    //     to what I'm suggesting here.
}