Skip to content

No compile error thrown when this referenced before call to super completes #8060

Description

@tomdye

TypeScript Version:

1.8.9

Code
JSFiddle: https://jsfiddle.net/kitsonk/fs9t96ep/
TS Playground: http://goo.gl/X7cgvV

class A {
    constructor(fn: () => void) {
        fn.call(this);
    }
    foo: string = 'foo';
}

class B extends A {
    constructor() {
        super(() => {
            console.log(this);
        });
    }
    bar: string = 'bar';
}

const b = new B();

Expected behavior:
Typescript should guard against the use of this as the call to super has not completed. Thus it should not compile.

Actual behavior:
Typescript compiles this successfully. It works fine when the target is set to es5 but breaks the browser when target is set to es6.
Error is: VM89:55 Uncaught ReferenceError: this is not defined

Activity

  1. RyanCavanaugh commented on Apr 13, 2016

    @RyanCavanaugh
    Member

    This code isn't necessarily invalid -- it's entirely possible (i.e. we can't see its implementation) that the super constructor doesn't invoke the method immediately, in which case this would be valid to access at a later date.

  2. kitsonk commented on Apr 13, 2016

    @kitsonk
    Contributor

    Ryan Cavanaugh (@RyanCavanaugh) the ES6 code:

    class A {
        constructor(fn) {
            fn.call(this);
        this.foo = 'foo';
        }
    }
    
    class B extends A {
        constructor() {
            super(() => {
                console.log(this);
            });
        this.bar = 'bar';
        }
    }
    
    const b = new B();

    Fails in Chrome with:

    Uncaught ReferenceError: this is not defined
    

    And in Edge:

    SCRIPT5113: Use before declaration
    

    And Firefox:

    ReferenceError: |this| used uninitialized in arrow function in class constructor
    

    It is because it is a lambda function and therefore it is trying to access the lexical this, which isn't available until after super completes, irrespective of what is actually in the super constructor. Therefore, you cannot use lambda function as arguments of a super call if they reference this in ES6 (or all 3 browser manufactures have mis-implemented the spec).

  3. RyanCavanaugh commented on Apr 13, 2016

    @RyanCavanaugh
    Member

    This works for me in Chrome and Edge?

    image

    class A {
        constructor(fn) {
           this.foo = 'foo';
           this.print = fn;
        }
    }
    
    class B extends A {
        constructor() {
            super(() => {
                console.log(this.bar);
            });
            this.bar = 'bar';
        }
    }
    
    const b = new B();
    console.log(b.foo);
    b.print();
  4. kitsonk commented on Apr 13, 2016

    @kitsonk
    Contributor

    Hmmm... wow, ok, I can break it again if I call it within the constructor:

    class A {
        constructor(fn) {
           this.foo = 'foo';
           this.print = fn;
           fn();
        }
    }
    
    class B extends A {
        constructor() {
            super(() => {
                console.log(this.bar);
            });
            this.bar = 'bar';
        }
    }
    
    const b = new B();
    console.log(b.foo);
    b.print();

    The specific use case that Tom Dye (@tomdye) and I ran into was a situation where we were extending Promise and passing in a lambda function as the executor, which gets invoked immediately. So we were going merrily along, targeting ES5 and then when we tried to target ES6, it broke.

    But that is really difficult to guard against, I suspect.

  5. kitsonk commented on Apr 13, 2016

    @kitsonk
    Contributor

    Bryan Forbes (@bryanforbes) just suggest that given:

    class B extends A {
        constructor() {
            super(() => {
                console.log(this);
            });
        }
        bar: string = 'bar';
    }

    If the emit was:

    var B = (function (_super) {
        __extends(B, _super);
        function B() {
            var _this;
            _super.call(this, function () {
                console.log(_this);
            });
            _this = this;
            this.bar = 'bar';
        }
        return B;
    }(A));

    Then the runtime behaviour of ES5 would match ES6.

  6. added
    BugA bug in TypeScript
    and removed
    By DesignDeprecated - use "Working as Intended" or "Design Limitation" instead
    on Apr 13, 2016
  7. RyanCavanaugh commented on Apr 13, 2016

    @RyanCavanaugh
    Member

    That seems like a good solution, though I'm slightly afraid of breaking existing working code.

  8. tomdye commented on Apr 13, 2016

    @tomdye
    Author

    It will only break code that's invalid ES6 in the first place though. If they were ever to try setting the compile target to 'es6' they would see this error at runtime. Must be better to catch it at compile time now rather than down the line should they choose to compile to 'es6' at a later date.

  9. RyanCavanaugh commented on Apr 13, 2016

    @RyanCavanaugh
    Member

    The problem is we won't be catching at compile time -- people will upgrade to the next TypeScript version, recompile to ES5, and their code will start breaking at runtime with some very confusing errors and emit that looks like a bug.

  10. kitsonk commented on Apr 13, 2016

    @kitsonk
    Contributor

    Yes, agreed. It is clearly a breaking change, but one that preserves the runtime semantics. Maybe there is room for a linter to flag uses of lexical this within the scope of a super call (Adi Dahiya (@adidahiya)), since it could be uninitialized?

  11. added
    SuggestionAn idea for TypeScript
    and removed
    BugA bug in TypeScript
    on Jun 7, 2016
  12. RyanCavanaugh commented on Jun 9, 2016

    @RyanCavanaugh
    Member

    Accepting PRs once the new transform-based emitter merges. Warning, this is likely a difficult change.

  13. added this to the milestone on Jun 9, 2016
  14. kitsonk commented on Nov 4, 2016

    @kitsonk
    Contributor

    Just a note on this, the emit in master (TS 2.1) now breaks these examples in ES5 at run-time, so this now has parity. It still does not warn you that it is invalid.

    Given:

    class A {
        constructor(fn: () => void) {
            fn.call(this);
        }
        foo: string = 'foo';
    }
    
    class B extends A {
        constructor() {
            super(() => {
                console.log(this);
            });
        }
        bar: string = 'bar';
    }

    The emit is...

    Version 2.1.0-dev.20161103

    var __extends = (this && this.__extends) || function (d, b) {
        for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
    var A = (function () {
        function A(fn) {
            this.foo = 'foo';
            fn.call(this);
        }
        return A;
    }());
    var B = (function (_super) {
        __extends(B, _super);
        function B() {
            var _this = _super.call(this, function () {
                console.log(_this);
            }) || this;
            _this.bar = 'bar';
            return _this;
        }
        return B;
    }(A));

    Version 2.0.6

    var __extends = (this && this.__extends) || function (d, b) {
        for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
    var A = (function () {
        function A(fn) {
            this.foo = 'foo';
            fn.call(this);
        }
        return A;
    }());
    var B = (function (_super) {
        __extends(B, _super);
        function B() {
            var _this = this;
            _super.call(this, function () {
                console.log(_this);
            });
            this.bar = 'bar';
        }
        return B;
    }(A));
  15. zemlanin commented on Nov 19, 2016

    @zemlanin

    One more case (https://gist.github.com/zemlanin/a306e8498d05c923cac405c047014029):

    // this.ts
    class A {
        constructor(protected cb: () => void) { }
    }
    
    class Greeter extends A {
        constructor() {
            // ^ there should be `var _this = this;`
            super(() => { this })
        }
    }
    // compiled_with_ts2_0_10.js
    var __extends = (this && this.__extends) || function (d, b) {
        for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
    var A = (function () {
        function A(cb) {
            this.cb = cb;
        }
        return A;
    }());
    var Greeter = (function (_super) {
        __extends(Greeter, _super);
        function Greeter() {
            var _this = this;
            // ^ there should be `var _this = this;`
            _super.call(this, function () { _this; });
        }
        return Greeter;
    }(A));
    // compiled_with_ts2_1_1.js
    var __extends = (this && this.__extends) || function (d, b) {
        for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
        function __() { this.constructor = d; }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };
    var A = (function () {
        function A(cb) {
            this.cb = cb;
        }
        return A;
    }());
    var Greeter = (function (_super) {
        __extends(Greeter, _super);
        function Greeter() {
            return
            // ^ there should be `var _this = this;`
            _super.call(this, function () { _this; }) || this;
        }
        return Greeter;
    }(A));
  16. added a commit that references this issue on Nov 20, 2016
  17. Harley-Adams commented on Mar 3, 2017

    @Harley-Adams
    Member

    I've also found this with creating an arrow function before calling super.

    class Foo extends Object {
        public constructor() {
            const test = () => {
                console.log(this);
            };
            test();
            super();
        }
    }

    You can see here that we the output has var _this = this; which shouldn't be valid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions