Skip to content

Allow to call getter/setter logic When subclassing #338

Description

http://typescript.codeplex.com/workitem/645

At the moment it's not possible to call super getter/setter when overriding properties:

class A {
    private _property: string;
    get property() { return this._property; }
    set property(value: string) { this._property = value }
}

class B extends A {
    set property(value: string) {
        super.property = value + " addition";
    }
}

Activity

  1. RyanCavanaugh commented on Aug 4, 2014

    @RyanCavanaugh
    Member

    The problem is that we don't surface the fact that a property is a getter/setter in the type information, and it's impossible to emit code here that would work even if the base class's definition was not a getter/setter. Using 'virtual' properties in JavaScript is generally not a good idea for this reason.

    /* MyDeclFile.d.ts */
    declare class A {
      x: number; // actually implemented as getter/setter
      y: number; // actually implemented as field
    }
    
    /* MyConsumer.ts */
    class B {
       get x() { return super.x; } // Could theoretically emit code for this
       get y() { return super.y; } // Cannot work; there is no 'super.y' to refer to
    }
  2. fdecampredon commented on Aug 10, 2014

    @fdecampredon

    It's part of es6 class, so it seems pretty important to me, also generating something that throws an error in case of missing super setter and return undefined in case of missing super getter (like what traceur does https://github.com/google/traceur-compiler/blob/master/src/runtime/classes.js#L43-L60) seems pretty much standard and compliant with what future es6 engine will do.

  3. RyanCavanaugh commented on Aug 11, 2014

    @RyanCavanaugh
    Member

    The point about ES6 is very important. We'll need to figure out what that means.

  4. 51 remaining items

  5. Neewd commented on Sep 28, 2017

    @Neewd

    Hi,

    Any update about this issue ?

  6. kriszyp commented on Oct 5, 2017

    @kriszyp
    Contributor

    For the record, when we transitioned from Babel to TypeScript, I wrote this custom transformer + runtime patch. It's a bit of hack from a transformation perspective, but I believe it is semantically correct in behavior (and preserved our super accessor functionality as compiled with babel):
    (before) transformer:

    const ts = require('typescript')
    
    function visitor(ctx, sf) {
    
      function visit(node) {
        if (ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.EqualsToken && ts.isPropertyAccessExpression(node.left) && node.left.expression.kind === ts.SyntaxKind.SuperKeyword) {
          // super.property = value
          // transform to:
          // Object.superSet(_super.prototype, propertyName, this, value)
          return ts.createCall(
            ts.createPropertyAccess(
              ts.createIdentifier('Object'),
              'superSet'),
            [],
            [ts.createSuper(), ts.createLiteral(node.left.name), ts.createThis(), node.right])
        }
        if (ts.isPropertyAccessExpression(node) && node.expression.kind === ts.SyntaxKind.SuperKeyword &&
            // make sure we *don't* transform super.method() calls
            !(ts.isCallExpression(node.parent) && node.parent.expression === node)) {
          // super.property
          // transform to:
          // Object.superGet(_super.prototype, propertyName, this)
          return ts.createCall(
            ts.createPropertyAccess(
              ts.createIdentifier('Object'),
              'superGet'),
            [],
            [ts.createSuper(), ts.createLiteral(node.name), ts.createThis()])
        }
        return ts.visitEachChild(node, visit, ctx)
      }
      return visit
    }
    
    module.exports = function(/*opts?: Opts*/) {
      return (ctx) => {
        return (sf) => ts.visitNode(sf, visitor(ctx, sf))
      }
    }
    

    runtime patch:

    Object.superGet = (superPrototype, name, instance) => {
      do {
        let descriptor = Object.getOwnPropertyDescriptor(superPrototype, name)
        if (descriptor) {
          return descriptor.get ? descriptor.get.call(instance) : descriptor.value
        }
      } while ((superPrototype = Object.getPrototypeOf(superPrototype)))
    }
    
    Object.superSet = (superPrototype, name, instance, value) => {
      do {
        let descriptor = Object.getOwnPropertyDescriptor(superPrototype, name)
        if (descriptor) {
          return descriptor.set && descriptor.set.call(instance, value)
        }
      } while ((superPrototype = Object.getPrototypeOf(superPrototype)))
      Object.defineProperty(instance, name, {
        value,
        configurable: true,
        enumerable: true,
        writable: true
      })
    }
    

    The runtime semantics require no knowledge of types or super class implementations. Obviously to put this in TypeScript proper, you would want that code to be generated, and/or in tslib, not patched on Object, though. As a before transformer, this relies on the fact that generating a super token will get transformed to _super.prototype in the later downlevel transformers. Naturally, would greatly prefer this in TypeScript itself, so maybe this will help.

  7. brianmhunt commented on Nov 9, 2017

    @brianmhunt

    Thanks for reopening, Ryan Cavanaugh (@RyanCavanaugh) — this is a problematic issue and readily worked around, per e.g. the patch by Kris Zyp (@kriszyp)

  8. added
    DeclinedThe issue was declined as something which matches the TypeScript vision
    and removed on Nov 28, 2017
  9. RyanCavanaugh commented on Nov 28, 2017

    @RyanCavanaugh
    Member

    Long story short - after five years of not having this and people surviving despite it, it seems like ES6 adoption is catching up fast enough that the narrow window of people who target ES5 (rather than 3 for maximum compat or 6 for smallest emit delta) is closing to the point where we don't want to accept a big chunk of helper emit at this point. Open to data otherwise but it just doesn't seem common enough to warrant a big change in the emitter.

  10. brianmhunt commented on Nov 28, 2017

    @brianmhunt

    Just a data point: This is a blocker to using Typescript to compile Knockout.js version 4. The depth of workaround needed is uncertain, but it feels problematic.

    Cross-linking knockout/tko#21

  11. thomashilzendegen commented on Dec 15, 2017

    @thomashilzendegen

    I'm just curious, but when I put the following code into the playground, the output JS seems legit to me (and it works):

    class Foo {
        get bar() {
            return 'foo';
        }
    }
    
    class Bar extends Foo {
        get bar() {
            return super.bar + 'bar';
        }
    }
    
    const x = new Bar();
    alert(x.bar);
    

    Does this mean, access to super's property works, but the compiler is still complaining?

  12. brianmhunt commented on Dec 15, 2017

    @brianmhunt

    Thomas den Hollander (@ThomasdenH) It's references to this that are broken. Try:

    class Foo {
        constructor () { this.x = 'dee' }
        get bar() {
            return 'foo' + this.x;
        }
    }
    
    class Bar extends Foo {
        get bar() {
            return super.bar + 'bar';
        }
    }
    
    const foo = new Foo()
    const bar = new Bar();
    alert(`foo: ${foo.bar}, bar: ${bar.bar}`)

    The output above is foo: foodee, bar: barundefinedbar, but when run in an ES6 engine (i.e. the correct outcome) it is foo: foodee, bar: foodeebar.

  13. thomashilzendegen commented on Dec 15, 2017

    @thomashilzendegen

    Brian M Hunt (@brianmhunt) Brrr, I see. Good catch.

  14. SlurpTheo commented on Dec 15, 2017

    @SlurpTheo

    The output above is foo: foodee, bar: barundefinedbar, but when run in an ES6 engine (i.e. the correct outcome) it is foo: foodee, bar: bardeebar.

    Brian M Hunt (@brianmhunt) Little typo there:

    The output above is foo: foodee, bar: fooundefinedbar, but when run in an ES6 engine (i.e. the correct outcome) it is foo: foodee, bar: foodeebar.

  15. brianmhunt commented on Dec 15, 2017

    @brianmhunt

    SlurpTheo Yes, you're correct; edited my comment to fix; thanks!

  16. locked and limited conversation to collaborators on Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    DeclinedThe issue was declined as something which matches the TypeScript visionSuggestionAn idea for TypeScript

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions