New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"constructor" is enumerable when targetting es5 #104
Comments
Well, perhaps it shouldn't even be defined as |
Thoughts here @rbuckton? |
Constructors are writable in ES2015+: If we did this we would have to feature test for whether we can actually use 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 __());
}; |
Considering how similar this is to the new 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 |
Is this something that will be fixed, or are you worried about the subtle breaking behavior? |
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:
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:Notice that the current implementation doesn't use
defineProperty
and is instead assigning thethis.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.
The text was updated successfully, but these errors were encountered: