TypeScript: [Proposal] Limit the compiler error about overriding the base class property with getter/setter accessors pair to the `useDefineForClassFields : true` case only

As suggested by @sandersn, opening a sibling issue to track the proposal from the https://github.com/microsoft/TypeScript/pull/37894

Topic

Currently, when a property in the base class is overridden with a getter/setter pair, compiler always generates an error: Playground link

class BaseClass {
    prop        : string        = 'base_class'
}

class SubClass extends BaseClass {
    _prop       : string        = 'sub_class'

    /*
       ERROR: 'prop' is defined as a property in class 'BaseClass', but is overridden here in 'SubClass' as an accessor.
    */
    get prop () : string {
        return this._prop
    }
    set prop (value : string) {
        this._prop = value
    }
}
// try running this snippet with `useDefineForClassFields : true` and `useDefineForClassFields : false`
console.log((new SubClass).prop)

This error is motivated by the fact, that in modern JavaScript, class fields have DEFINE semantic and the above example won’t work as intuitively expected if useDefineForClassFields compiler option is set to true.

The property access augmentation pattern

Need to note, that historically nobody was ever using the DEFINE semantic for class properties. Pretty much all the JavaScript and TypeScript code in the world currently assumes the SET semantic for class fields.

SET semantic makes it possible to use the property access augmentation pattern. Its main use case is to trigger some arbitrary code on property read/write. This is very useful for variety of purposes and is done by simply overriding the property of the base class with getter/setter accessors pair.

For example, lets say we have some 3rd party library with a class with parent property.

class ExternalClass {
    parent      : this      = undefined
    children    : this[]    = []

    appendChild (child : this) {
        child.parent    = this        ​
   ​}
}

In the whole library, the parent is assumed to be a regular property and is accessed with dot notation.

Now lets say we want to plug this 3rd party library in our codebase, which has reactive capabilities. We need to track the access to the parent property and trigger some extra code on property write:

const reactive : any = undefined
type Reactive<V> = V

class OurClass extends ExternalClass {
   ​@reactive()
   ​reactiveParent     : Reactive<this>     = undefined

   ​private _parent : this
   ​get parent () : this {
       ​return this._parent
   ​}
   ​set parent (value : this) {
       ​this._parent = value
       ​this.reactiveParent = value
   ​}
}

With this simple subclass, we can use the 3rd party library code unchanged and mirror the writes to the parent property to the reactiveParent (which is plugged into the rendering process elsewhere).

DEFINE / SET semantic

TypeScript and JavaScript historically were using SET semantic for class fields. Pretty much all the code in the world assumes it. However, TC39 committee decided that class fields should use DEFINE semantic and to provide the way to migrate to new behavior, TypeScript introduced the new compiler config, useDefineForClassFields, disabled by default, because its a breaking change. Obviously, the “classic”, historical behavior will need to be supported for a very long time.

With this option enabled, the “property access augmentation” pattern becomes invalid, as demonstrated by the example in the beginning (see however, the “Additional considerations” below). It is probably reasonable to generate a compiler error being discussed in this case.

With this option disabled, the pattern remains valid. In this case there are no reasons to limit the language expressiveness and restrict the user from using the pattern.

Proposal

Proposal is to generate the compiler error being discussed only for the “DEFINE” semantic (useDefineForClassFields : true) case.

Additional considerations

There’s a promise from TC39 committee to provide an opt-in escape route to the “classic” SET semantic for class fields, using a decorator. This means, that the “property access augmentation” pattern will probably remain valid in JavaScript, even for native class fields, a user will just need to opt-in for it, using the decorator.

Ideally, compiler will need to be smart, to determine which semantic the field is actually using, to avoid the needless restrictions. Perhaps the nature of this error is that it should be handled better by the linter, rather than compiler.