"constructor" is enumerable when targetting es5

[jdom]

Hi, I've noticed that the behavior of "__extends" with regards to defining the "constructor" property is unexpected when you target "es5".

Try the following example in the TypeScript playground:

class TestSuper { }
class TestSub extends TestSuper { }

const properties = [];
for (const p in new TestSub()) {
    properties.push(p);
}
console.assert(properties.length === 0, "Should not have any enumerable properties");
console.assert(properties.indexOf("constructor") === -1, "Should not have constructor as an enumerable property");

If you compile this with ES2015, then classes and inheritance are natively supported, and running the code doesn't print anything to the console (as "constructor" isn't enumerable). If you change the target to "es5", then you'll see stuff in the console, since "constructor" is now enumerable.

The fix should be easy, it's a matter of redefining __extends as follows:

    __extends = function (d, b) {
        extendStatics(d, b);
        function __() { Object.defineProperty(this, "constructor", { configurable: true, value: d, writable: true }); }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };

Notice that the current implementation doesn't use defineProperty and is instead assigning the this.constructor member directly.

Is this a behavioral change that we'd like? I can submit a PR for it. But since it would be a breaking change, I don't know what would be the decision here. As additional input, we've been using an updated version of __extends with this fix for several years in the Azure Portal.

[jdom]

Well, perhaps it shouldn't even be defined as writable: true either, as I believe the native constructors are readonly, but please advice on whether something like this would be useful to fix

[DanielRosenwasser]

Thoughts here @rbuckton?

[rbuckton]

Well, perhaps it shouldn't even be defined as writable: true either, as I believe the native constructors are readonly, but please advice on whether something like this would be useful to fix

Constructors are writable in ES2015+:

If we did this we would have to feature test for whether we can actually use defineProperty:

    var __setConstructor = Object.create ? function(o, d) {
        Object.defineProperty(o, "constructor", { writable: true, configurable: true, value: d });
    } : function (o, d) {
        o.constructor = d;
    };
    __extends = function (d, b) {
        extendStatics(d, b);
        function __() { __setConstructor(this, d); }
        d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
    };

[rbuckton]

Considering how similar this is to the new __setModuleDefault I wonder if we don't just need a shortcut for "defining" things:

var __define = Object.create ? function (o, k, v, f) {
    Object.defineProperty(o, k, {
        enumerable: !!(f & 1),
        configurable: !!(f & 2),
        writable: !!(f & 4),
        value: v
    });
} : function (o, k, v) {
    o[k] = v;
};

And replace __setModuleDefault(result, mod) with __define(result, "default", mod, 1) and __setConstructor with __define(this, "constructor", d, 6).

[jdom]

Is this something that will be fixed, or are you worried about the subtle breaking behavior?