Typescript: Don't require to implement optional abstract properties

Created on 18 Sep 2020  路  10Comments  路  Source: microsoft/TypeScript

Search Terms

abstract optional override

Suggestion

If in abstract class abstract property is marked as optional, allow child classes not to implement it.
So I suggest to remove an error for property x of class D in the following code:

https://www.typescriptlang.org/play?ts=4.0.2#code/IYIwzgLgTsDGEAJYBthjAgggg3gKAUIQAcoB7CAU3koBMFRIZ4EpLhayA7ZATwQAeAfgBcCLgFcAtiEpQCRUhWpV6jaHERsO3Pgl5jJMuQgA+CCV1qUAZgEsudPAsLEJIZHdgJOAZSkQABYAFACUuC5EhLDcYGTIlAB0yGQA5sFBdmCJAgA0CJnZvKGRAL545XgoaBgAIgiUAlRWGNj4UUpUNPTanDz8-AC8CACMAAwVzo4A7gi1YYl+ASGhQA

abstract class A {
    protected abstract readonly x?: number
    protected abstract readonly y: number | undefined

    public doSmth() {
        console.log(this.x, this.y)
    }
}

class D extends A {
    protected readonly y = 10
}

new D().doSmth()

But property y still must be implemented.

Note that we already can do some sort of it

https://www.typescriptlang.org/play?ts=4.0.2#code/JYOwLgpgTgZghgYwgAgJLIN4Chm+QBygHt9owBPAYThAGUBrYfAfgC5kQBXAWwCNoceQiTLkAspwDOYVN3wAbCNwjh2XPtGQAfZJxAATCDFAR9g3OeTKwACyL7qdRiwAUASnYA3IsDN4rELb2EtKyCkoqYO5ePmYAvlhYcLzSUIhgyAjycJKSyACCmJbCpFAUjgxMbBw8-FCWyanpBMSlFCEycorKqjUaUNq6BkYmfniNYGkIGSWiHWHdkQAqRETV6nWJ-tZ2DjSVrh7I3r4NKZPNO8FSneE9UUcnY7gTUxlX+vNdEeAra9HHWJYBJYLI5PKUQoQAAekAMeUK2AA9AAqSwAOSIIAAtK9mmDcsgAOSQonIfRECB5EBEDLAb73ZCgGzQYCQfTIPHTAL9YkfL53SJkmDEbiZbKEon5IkAOhcACYAKwARkVbgxWNx5ze4vBxNJ5Mp1NpTIZkSZIBZUDZpk52suSjqfMCuwFi1+q2Fot1kulcqVqvV-kxOK5GQJeRJ0sNVI4JvpgvAFqtNo5YZ5TqJszK4huCx+YC9RDFEeJfoVKrVGtD9u5pajZIpsZpdLNSeZrPZdqa3OUvKzrTmebbYD+RZLEsj5YDVZRSOBiVLlHQCfdYDy6GRaP8lEn+tQZNACCIUCgEGm8nIpsT64tkFgiBQRIPMssS3IpH3h7y3GAuVAADmyC2CgMBEPI8hEAA7oBLQiGUwCxiKxbAR+T4Huw2btMON4ADQBEEnw4WuCoACzygADOqc7AkAA

interface I {
    propertyCanSkip?: number
    propertyMustImplement: number | undefined

    methodCanSkip?(): void
    methodMustImplement(): void
}

abstract class A {
    propertyCanSkip?: number
    abstract propertyMustImplement: number | undefined
    abstract propertyMustImplementToo?: number

    methodCanSkip?(): void
    abstract methodMustImplement(): void
    abstract methodMustImplementToo?(): void
}

class CA extends A {
/*
    Non-abstract class 'CA' does not implement inherited abstract member 'methodMustImplement' from class 'A'.(2515)
    Non-abstract class 'CA' does not implement inherited abstract member 'methodMustImplementToo' from class 'A'.(2515)
    Non-abstract class 'CA' does not implement inherited abstract member 'propertyMustImplement' from class 'A'.(2515)
    Non-abstract class 'CA' does not implement inherited abstract member 'propertyMustImplementToo' from class 'A'.(2515)
*/
}

class CI implements I {
/*
    Class 'CI' incorrectly implements interface 'I'.
    Type 'CI' is missing the following properties from type 'I': propertyMustImplement, methodMustImplement(2420)
*/
}

But there is a set of problems for properties

We have two ways to override property (via property declaration or via getter and setter). And now (in TS4) the limitation have changed. For nonabstract property base class always defines how it should be implemented in children.

Let's look what implementations are possible (don't forget about useDefineForClassFields compiler flag that makes it more important):

Code | Property can be omitted | Child can implement as property | Child can implement as get/set
------------ | ------------- | ------------- | -------------
propertyCanSkip?: number | Yes | Yes | No
abstract propertyMustImplement: number \| undefined | No | Yes | Yes (except https://github.com/microsoft/TypeScript/issues/40632)
abstract propertyMustImplementToo?: number | No | Yes | Yes (except https://github.com/microsoft/TypeScript/issues/40632)
get getter?(): number | N/A | N/A | N/A
abstract get getterMustImplement(): number \| undefined | No | No | Yes
abstract getterToo?(): number | N/A | N/A | N/A

It's easy to see, that if the property should really be optional, there is only one way to make it such which will not allow to implement it as getter and setter. But we have 2 absolutely identical lines with optional and nonoptional abstract property. I see no sense for them to be synonyms as ? in the 3rd line definitely says that the property _is optional_, but doesn't give me ability to make so in further code.

So I propose to change this table in following way:

Code | Property can be omitted | Child can implement as property | Child can implement as get/set
------------ | ------------- | ------------- | -------------
propertyCanSkip?: number | Yes | Yes | No
abstract propertyMustImplement: number \| undefined | No | Yes | Yes
abstract propertyCanSkipToo?: number | Yes | Yes | Yes
get getter?(): number | N/A | N/A | N/A
abstract get getterMustImplement(): number \| undefined | No | No | Yes
abstract get getterCanSkipToo?(): number | Yes | No | Yes

Abstract getter is NOT a part of this feature request, just shown for consistency.

Use Cases

Provide ability to list and use for reading an optional property in abstract class without limiting a way of its implementation in child classes. Such problem occured in a real project because of migration from TS3 to TS4. Before that it was possible, but because of breaking changes of TS4 it's not anymore.

https://www.typescriptlang.org/play?ts=4.0.2#code/IYIwzgLgTsDGEAJYBthjAgggg3gKAUIQAcoB7CAU3koBMEpLhayA7ZATwTAFsIALAPwAuBKwCuPEJSh4CRYuJDIAlrAQAzABQBKXPKJFYbMGWSUAdMjIBzLQJVgLvAToMIAvni94UaDABCCJQAHlSstBjY+IYu-AgAvAgAjAAM3nJ+6AgAwsFhlBFR+oY2lIhxurgM5eJQrAgAssACFjARZDxVXj5ZGAAi+eGRWPo+rJQA7ggBuhbabhPTOXMLeEsI-au6QA

abstract class A {
    protected readonly smth?: number

    public f() {
        console.log(this.smth)
    }
}

class B extends A {
    smth = 10
}

class C extends A {
    get smth() { return Math.random() }
}

class D extends A {
}

new B().f()
new C().f()
new D().f()

Examples

See above.

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.

Why it's not a breaking change?

It changes behavior of existing construction, but it's not a breaking change in terms of code.

If you had your code working and you have

abstract propertyMustImplementToo?: number

in it, that means that you implemented this property in all child classes. So after its meaning changes all you code keeps being valid and compiles into absolutely the same javascript code as before. Nothing changed.

At the same time, for further development you have to decide whether you want to allow child classes to skip the property or not. If yes, or you don't care - keep it with ? as it is. If no then update it to

abstract propertyMustImplementToo: number | undefined

without any other changes needed.

Related Issues:

https://github.com/microsoft/TypeScript/issues/6413
https://github.com/microsoft/TypeScript/issues/22939

Awaiting More Feedback Suggestion

Most helpful comment

I said

abstract class C { p?: number }
abstract class K { abstract p?: number }

are checked the same today (and emitted, but that's bug #40699)

class D extends C { } // doesn't need to be abstract, p is optional
class B extends K { } // currently: B must be abstract

This issue requests that B be valid code, and not required to be abstract. But that's the same as D.

All 10 comments

Discussed this with @sandersn a bit and we think this is reasonable, but also that the current behavior is also very defensible. In general we're stare decisis for longstanding behavior that hasn't received any other feedback, so would prefer to leave this alone unless there's strong evidence that the majority of the people using this particular set of modifiers feels the same way.

Another point from our discussion; if #40632 is fixed, these two programs emit different code with useDefineForClassFields: true:

abstract class C { p?: number }

emits a defineProperty.

abstract class C { abstract p?: number }

would not emit a defineProperty.

Right now these two programs both emit the [[Define]], and they both check the same way, so you can get the desired effect today by leaving off abstract.

@sandersn why do you compare

abstract class C { p?: number }
abstract class C { abstract p?: number }

they are already different.

But

abstract class C { abstract p?: number }
abstract class C { abstract p: number | undefined }

currently are the same and won't become different after the fix.

so you can get the desired effect today by leaving off abstract.

I don't understand it.

Here is the code that is valid for TS3 (playground)

abstract class A {
    protected readonly smth?: number

    public f() {
        console.log(this.smth)
    }
}

class B extends A {
    smth = 10
}

class C extends A {
    get smth() { return Math.random() }
}

class D extends A {
}

new B().f()
new C().f()
new D().f()

If I switch to TS4 (playground), I'll get an error:

'smth' is defined as a property in class 'A', but is overridden here in 'C' as an accessor. (2611)

I expected abstract to solve the problem, but if I add it, the error above really disappears, but the other one occurs (playground):

Non-abstract class 'D' does not implement inherited abstract member 'smth' from class 'A'. (2515)

You say I can remove abstract to get it working. Remove from where?


By the way, another moment you've shown:

with useDefineForClassFields: true:

abstract class C { p?: number }

emits a defineProperty.

Is it really a good idea?

  1. It's strange to mark as optional something, that would be created in the same statement where it is marked as optional.
  2. Such generation will completely disallow having real optional fields in classes. Any declaration (except of abstract) will produce a property, but without declaration it's impossible to use the property. So you'll never can create no property.

Seems like for this code the same thing I wrote should be applied:

p?: number;              // Optional property - don't create
p: number | undefined;   // Nonoptional property - create

Typescript doesn't distinguish between p?: number and p: number | undefined in properties. The first is a shortcut for the second.

@sandersn
Does聽that聽mean that聽with a聽type聽like:

interface Foo {
     bar: string;
     baz: string | undefined;
}

It鈥檚聽valid聽to聽do:

const foo: Foo = { bar: "bar" };

By聽omitting baz even聽with strictNullChecks?

No, I am wrong. p?: number is equivalent to p?: number | undefined BUT is different from p: number | undefined.

I said

abstract class C { p?: number }
abstract class K { abstract p?: number }

are checked the same today (and emitted, but that's bug #40699)

class D extends C { } // doesn't need to be abstract, p is optional
class B extends K { } // currently: B must be abstract

This issue requests that B be valid code, and not required to be abstract. But that's the same as D.

@sandersn I don't understand you still. Here is code for TS3 - how do you propose to update class A without changing B, C and D so that the code will be valid for TS4? Full example, please.

B vs C is a separate issue. For B and D, you don't need to update A if you want smth to be optional. If you want smth to be required, make it abstract.

@sandersn I want optional field and all 3 classes compatible with A. For me it seems impossible now, isn't it?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

weswigham picture weswigham  路  3Comments

wmaurer picture wmaurer  路  3Comments

DanielRosenwasser picture DanielRosenwasser  路  3Comments

Zlatkovsky picture Zlatkovsky  路  3Comments

blendsdk picture blendsdk  路  3Comments