Static getter executed on class inheritance

[casser]

TypeScript Version:

1.7.5 / 1.8.0-beta / nightly (1.9.0-dev.20160217)

Code

class A{
    public static get foo(){
        return alert("should not execute getter, without accessing it !");
    }
}
class B extends A {}

Expected behavior:

Should not alert message above

Actual behavior:

Alerting message above

Link to Playground

[malibuzios]

Tried it on 1.9.0-dev.20160501 on ES5 target, same result, the _extends helper looks like this:

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 __());
};

I guess perhaps the assignment here could check if the member is a property and then use something like Object.getOwnPropertyDescriptor and then Object.defineProperty (or alternatively just Object.assign the whole thing if possible (but that wouldn't work with non-enumerable members, see below)).

Something like:

var __extends = (this && this.__extends) || function (d, b) {
    for (var p in b) 
        if (b.hasOwnProperty(p)) {
            var descriptor = Object.getOwnPropertyDescriptor(b, p);

            if (typeof descriptor.get === "function" ||
                typeof descriptor.set === "function") {
                Object.defineProperty(d, p, descriptor)
            }
            else {
                d[p] = b[p]; // This is only for performance, 'defineProperty' can be used for all cases
            }
        }
    function __() { this.constructor = d; }
    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};

In any case, with #3610 the current solution wouldn't work anyway, and for methods as well, as it only accounts for enumerable properties. They would need to use something like Object.getOwnPropertyNames(b) instead, so I guess this would be changed to something like:

var __extends = (this && this.__extends) || function (d, b) {
    var props = Object.getOwnPropertyNames(b);

    for (var i = 0; i < props.length; i++) { 
        var descriptor = Object.getOwnPropertyDescriptor(b, props[i]);
        Object.defineProperty(d, props[i], descriptor);
    }
    function __() { this.constructor = d; }
    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};

I'm surprised this hasn't been noticed yet?

Edit: I didn't actually notice this was about static methods. I was mostly referring to instance ones, so I'm not 100% sure if #3610 includes them as well - it is possible but I haven't checked..

[mhegazy]

This is a duplicate of #1520. please see reply in #1520 (comment).

[malibuzios]

@mhegazy

Seems like it would be fixed anyway when #3610 is introduced? (I mentioned that in my comment?)

[malibuzios]

Well, static getters/setters/methods seem to actually be enumerable based on my tests, so I'm not sure if it would be fixed any time soon... :'(

[malibuzios]

@mhegazy

I think you should seriously consider disallowing inheritance of classes with static getters and setters for ES3/ES5 targets. There's not really much of a point of having it if it doesn't actually work?

[mhegazy]

That would be a breaking change. Today you can do this by overriding the __extends helper, which is a supported scenario.

[malibuzios]

@mhegazy

How is it a breaking change to move from something that doesn't work to something that simply tells you it doesn't work? seems like an enhancement maybe?

Anyway, many people come from languages like C# where static classes are the main way to describe global functionality. I tend to use them this way myself. I do use static getters and setters and care a lot about ES5 compatibility.

(I noticed that in the compiler you almost don't use classes at all. Maybe if you've used it more extensively personally it would feel more important..?)

It could be true that the pattern is not statistically common but the more popular the language becomes, the more things that were once uncommon or seemed insignificant would start to come to light. I think this is a positive thing that is a sign of success, not failure. Also the more people start to truly understand and care about the language, they would want you to address some relatively minor things that might not seemed that important initially (why doesn't it error if..? etc), a lot of the time simply because they like to feel that they are using a 'polished' and 'well-rounded' product.

Also, people's use of the language will become more sophisticated with time as well, and things like generics, index signatures, static setter/getters, generic constraints etc. will become more widely used, so the impact of some of the 'holes' over there may also become more significant..

[malibuzios]

I don't think overriding helpers is something most people would want to deal with. It feels like something someone would do as an experiment maybe. In any case they would have to do this separately for every one of their projects and adapt it to the particular build system used in it, and somehow make sure it is evaluated before anything else (for example, I use a glob based build system so I don't currently have a precise order guarantee as the file list is auto-generated).

I could give you a pull request for:

Line 341 in emitter.ts

But I'm almost 100% sure it would be rejected, so I'm not even going to try..