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

A derived class is allowed be created without a parent's getter or setter. #47581

Open
dwilsonactual opened this issue Jan 24, 2022 Β· 8 comments
Open
Labels
Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Milestone

Comments

@dwilsonactual
Copy link

Bug Report

πŸ”Ž Search Terms

class extend override getter setter accessor

πŸ•— Version & Regression Information

  • This seems to occur on all versions on the Playground (3.9+)

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

class Parent {
    public get f() { return () => {}; }
}

class Child extends Parent {
    public set f(func: () => void) { }
}

const c = new Child();
c.f(); // runtime error because Child doesn't implement the getter for f

πŸ™ Actual behavior

Class Child is allowed to extend class Parent even though, by adding a setter for f, it discards the getter for f and therefore doesn't implement Parent's contract.

πŸ™‚ Expected behavior

TypeScript should emit an error because Child is not compatible with Parent.

@MartinJohns
Copy link
Contributor

Sounds like #30852.

@dwilsonactual
Copy link
Author

@MartinJohns I think you're right, the root of the problem is that when you add a setter, TypeScript assumes you have a getter whether one exists or not, and this is apparently by design.

@RyanCavanaugh RyanCavanaugh added Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript labels Jan 24, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jan 24, 2022
@RyanCavanaugh
Copy link
Member

It'd be nice to fix this but I'm not even sure how. The problem is that this isn't a soundness violation -- if we write out the getter that is implicitly present, there's no problem there either:

class Parent {
    public get f() { return () => {}; }
}

class Child extends Parent {
    public get(): never {
        throw new Error("readonly");
    }
    public set f(func: () => void) { }
}

An ad-hoc rule that a setter must have a matching getter if there's a property in the base class feels like a hack.

@dwilsonactual
Copy link
Author

I've convinced myself that the fix for now (if not forever) is to add this eslint rule to require getters and setters to always come in pairs "accessor-pairs": [ "error", { "setWithoutGet": true, "getWithoutSet": true }]

@OwenDelahoy
Copy link

OwenDelahoy commented Feb 8, 2022 β€’

In the following code, typescript expects a boolean, however at runtime it is actually undefined.

Playground Link

class Parent {
    protected _value = false;
    public get value() { return this._value; }
}

class Child extends Parent {
    public set value(value: boolean) { this._value = value; }
}

function f(): boolean {
    const obj = new Child();
    return obj.value;
}

function g(): true {
    const obj = new Child();
    obj.value = true;
    return obj.value
}

console.log(f()) // undefined
console.log(g()) // undefined

I think that most would expect Child to inherit Parent's getter and behave the same as the following code:
Playground Link

class Parent {
    protected _value = false;
    public get value() { return this._value; }
}

class Child extends Parent {
    public get value() { return this._value; }
    public set value(value: boolean) { this._value = value; }
}

function f(): boolean {
    const obj = new Child();
    return obj.value;
}

function g(): true {
    const obj = new Child();
    obj.value = true;
    return obj.value
}

console.log(f()) // false
console.log(g()) // true

@BlakeOne
Copy link

BlakeOne commented Jun 24, 2022 β€’

I think that most would expect Child to inherit Parent's getter

Agree completely that it’s expected to inherit the getter. IMO this makes overriding the setter unnecessarily difficult.

@Jack-Works
Copy link
Contributor

just being hit by this issue. The typeScript should error if the parent class has getter/setter but the child only have setter, because in most cases, people's intention is to "inherit" the getter from the parent class.

class Parent {
    get a() {
        return 1
    }
    set a(val: number) {}
}
class Child extends Parent {
    set a(val: number) {
        super.a = val
    }
}
const child = new Child()
child.a = 2
console.log('--- after write --')
console.log(child.a)
//          ts: child.a is number
//          js: child.a is undefined

@viktorpts
Copy link

viktorpts commented Oct 22, 2024 β€’

I'm joining this discussion, since if this behavior is by design, it's the same as reducing the accessibility of an overriden property compared to the base class (you're essentially creating a private getter). This not only breaks Liskov Substitution, but is also a compilation error if you attempt to do it explicitly:

class Parent {
    get a() { ... }
    set a(val: number) { ... }
}

class Child extends Parent {
    private override get a() { ... } // Compilation error
}

The child class should either have access to the parent's getter, or overriding only the setter should throw a compilation error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants