Skip to content

Optional abstract members #6413

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
tinganho opened this issue Jan 9, 2016 · 12 comments
Closed

Optional abstract members #6413

tinganho opened this issue Jan 9, 2016 · 12 comments
Labels
Needs More Info The issue still hasn't been fully clarified Suggestion An idea for TypeScript

Comments

@tinganho
Copy link
Contributor

tinganho commented Jan 9, 2016

I want to be able to make the implementation optional for abstract member, but it seems like it is not doable today.

abstract class Component {
    public abstract setText?(l: GetLocalization): void;
    public abstract bindInteractions?(): void;
}

I have some component classes and they MUST extend the base component class and optionally implement certain methods with a certain signature. Right now, I can only rely on good documentation.

class Post extends Component {
    public setText(l: GetLocalization): void {
        //...
    }
    // Though I don't want to implement bindInteractions method.
}

I want the power of auto-completion to also kick in.

Why is this useful?

It is useful for classes with life cycle methods.

@RyanCavanaugh RyanCavanaugh added Needs More Info The issue still hasn't been fully clarified Suggestion An idea for TypeScript labels Jan 11, 2016
@RyanCavanaugh
Copy link
Member

What would be an observable difference between optional and abstract optional?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 11, 2016

you can do this today with class an interface merging:

interface Component {
   optionalMethod?(l: GetLocalization): void;
}
abstract class Component {
    public abstract mustImplement(): void;
}

class Post extends Component {
    public mustImplement(): void {
       if (this.optionalMethod) {
            // do something
       }
    }
}

@tinganho
Copy link
Contributor Author

@mhegazy your trick seem to have solve my issue.

Though I think defining members and having some of them on a interface declaration and some of them in the class declaration is quite confusing. Besides there is some life-cycle methods that I don't want to expose publicly. I guess your solution only works with public members?

What would be an observable difference between optional and abstract optional?

I guess this could be said with non-optionals as well? I guess the noticeable difference is that, access modifier can be set to private and protected.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 12, 2016

looking at the original post again, i do not think abstract makes sense here. marking a method as abstract says it is impossible to instantiate this class without implementing this method. if it is optional, i.e. the class can work just fine without instantiating the class. so what you need is either 1. do not mark it as abstract and give it some default implementation, or 2. declare it as a property and do not mark it as abstract either.

@tinganho
Copy link
Contributor Author

I think I'm fine with empty body methods if abstract cannot be optional.

Little bit OT, is there a reason you don't autocomplete methods on an extended class?

@vilicvane
Copy link

vilicvane commented Jun 23, 2017

@mhegazy Currently TypeScript supports marking an abstract as optional but does not allow concrete classes to omit the implementation, this looks weird to me:

abstract class Foo {
  abstract foo?(): void;
}

class Bar extends Foo {
  // ERROR: Non-abstract class 'Bar' does not implement inherited abstract member 'foo' from class 'Foo'.
}

And an abstract method, though optional, could suggest the user that "you may want to try out implementing this", instead of "what can I do, I've implemented all those abstract methods, but I still need some tweaks over there".

@valotas
Copy link

valotas commented Jul 9, 2017

With interface/class merging, the types of the optional function are not inferred:

interface Component<T> {
   optionalMethod?(l: T): void;
}
abstract class Component<T> {
    public abstract mustImplement(): void;
}

class Impl extends Component<Props> {
  mustImplement() {
    // do nothing
  }

  optionalMethod(p) {
    // error TS7006: Parameter 'p' implicitly has an 'any' type.
  }
}

shouldn't p type be inferred here?

@sapjax
Copy link

sapjax commented Jan 31, 2018

abstract class Component {
    optionalMethod?(l: GetLocalization): void;
    public abstract mustImplement(): void;
}

class Post extends Component {
    public mustImplement(): void {
       if (this.optionalMethod) {
            // do something
       }
    }
}

I just do not use abstract right now.

@laughinghan
Copy link

laughinghan commented Mar 23, 2018

If abstract overrides the ?-optionality (which IMO it shouldn't), can we at least make abstract method?(); be a syntax error or warning that the ? does nothing, and it should be made non-abstract or moved into a merged interface instead?

@laughinghan
Copy link

@valotas Parameter types of subclass methods don't ever appear to be inferred:

class Parent {
    optionalMethod?(s: string): void;
}
class Child extends Parent {
    optionalMethod(s) {
        s // (parameter) s: any
    }
}

abstract class AbstractParent {
    abstract abstractMethod(s: string): void;
}
class ConcreteChild extends AbstractParent {
    abstractMethod(s) {
        s // (parameter) s: any
    }
}

interface Iface {
    method(s: string): void;
}
class Klass implements Iface {
    method(s) {
        s // (parameter) s: any
    }
}

So I don't think that's because of the interface/class merging.

@karol-depka
Copy link

karol-depka commented Jul 3, 2018

What helped for me is just to make them optional (?) but without abstract. And without body:

E.g.

export abstract class SyncRemoteConnector<TS> {

  upload?(syncable: TS): Observable<any>

  downloadAll?(): Observable<Array<TS>>
}

@Zarel
Copy link

Zarel commented Dec 28, 2020

If abstract overrides the ?-optionality (which IMO it shouldn't), can we at least make abstract method?(); be a syntax error or warning that the ? does nothing, and it should be made non-abstract or moved into a merged interface instead?

I think this could be useful for situations where you explicitly want the child class to say method: undefined; so you don't forget about the method.

But I think the error message should hint that you might have meant method?(); instead of abstract method?();.

jonseitz added a commit to seas-computing/course-planner that referenced this issue Sep 24, 2021
I know, I know. It's weird to have the populate() method in a
PopulationService be optional. HOWEVER! there are a number of tables
that we're populating through other services (e.g., meetings, course
instances) and we may need to split those out into separate drop()
functions so we can prevent deadlocks.

Apparently there isn't a way to define an optional abstract method in an
abstract class, but if we drop the abstract keyword from the method we
can make it optional and it will still enforce the type rules on
parameters and return values.

See: microsoft/TypeScript#6413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

9 participants