Override keyword should not be required on optional base type members, without implementations.

[arciisine]

Bug Report

🔎 Search Terms

override keyword optional

🕗 Version & Regression Information

• This is new functionailty released in 4.3.0

⏯ Playground Link

Playground link with relevant code

💻 Code

abstract class BaseType {
    abstract requiredSettings(): object;
    customInit?(): Promise<void>;
}

class Specialization extends BaseType {
    
    requiredSettings() {
        return {};
    }

    async customInit() {
        // .. do something
    }
}

🙁 Actual behavior

When using the noImplicitOverride flag, an optional base type field, with no implementation, requires the override keyword. By contrast abstract methods do not require the override keyword.

🙂 Expected behavior

An optional field, with no implementation in the base class should not require the override keyword.

[RyanCavanaugh]

(Crossposting this comment) This is intentional. The mental model to have in place here is, "If I typo this method name, what will happen?"

If you typo the name of an abstract member (assuming you're in a concrete class), you'll get an error, because you failed to implement all abstract members. override isn't necessary to prevent you from making this mistake, so you don't need to write override in this case.

If you typo the name of an optional member, you'll now have calculateHeight and cacluateHeight, and the latter will just go silently uncalled forever. override is necessary to prevent you from making this mistake, so when overriding an optional member, you should be writing override.

[arciisine]

@RyanCavanaugh thanks for the reply. I definitely understand the design goal of explicit declaration here. The challenge I'm seeing is the mismatch between class members that are optional, and interface methods that are optional. It appears interfaces are not held to the same standard as classes, and abstract class members are treated differently than non-abstract.

Below will actually throw a compilation error, as override is not supported on interface methods. I'm not sure if this is a gap, but it seems inconsistent from the perspective of abstract classes and interfaces achieving similar goals.

interface BaseType {
    customInit?(): Promise<void>;
}

class Specialization implements BaseType {
    
    override async customInit() {

    }
}

[RyanCavanaugh]

It's intentional that override doesn't apply to implements clauses, since you're not overriding anything.

[arciisine]

Right, but I'd argue that an optional member on an abstract class is not being overridden either, since it does not have an implementation. The scenario you provided with calculateHeight and cacluateHeight, should apply equally to an interface as much as it does to a base class, in terms of risk to incorrectly honoring the contract:

interface Calculatable {
   calculateHeight?(): number;
}

class Calculator implements Calculatable {
   /* overide*/  cacluateHeight() {
     return 0;
   }
}

vs

abstract class BaseCalculator {
   calculateHeight?(): number;
}

class Calculator extends BaseCalculator {
   overide cacluateHeight() {
     return 0;
   }
}

[arciisine]

@RyanCavanaugh From conversation it appears the goal of override is to support explicit declaration of specialized class implementation, excluding abstract methods. This is with the express goal to ensure that if the parent class modifies the function declaration, that the child class will fail compilation if it no longer honors the contract.

I still believe this desire/need can apply to interfaces as well, though the current issue has a work around. By declaring the optional method via an interface, the compiler will no longer consider the method a candidate for overriding, and so will not complain. It feels a little clunky, especially given abstract classes cannot implement interfaces with optional members.

interface OptionalBaseTypeMethods {
    customInit?(): Promise<void>;
}

abstract class BaseType {
    abstract requiredSettings(): object;
}

const BaseTypeClass = BaseType as (typeof BaseType & OptionalBaseTypeMethods);

class Specialization extends BaseTypeClass {
    
    requiredSettings() {
        return {};
    }
    async customInit() {

    }
}

[typescript-bot]

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.