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
Impossible to define static 'length' function on class #442
Comments
I ran into this recently. :( |
Also a few others are disallowed e.g. the following code should error (if we do this feature): class C {
static name = 'something';
static arguments = 'args';
static caller = 'caller';
}
console.log(C.name); // 'C'
console.log(C.arguments); // null
console.log(C.caller); // null Not all of these are stardard however. |
I don't think the properties |
@tinganho agreed |
Ok, but since people catch this kind of error from time to time I think that TS should notify them about doing something wrong. It because properties like |
May be because Lengh, Name are Already Defined. |
I'm going to try and work on this. Apart from what has already been mentioned, are there more property names that shouldn't be allowed? So far I see: What exactly should happen when someone tries to set a static property with one of the forbidden names? Compiler error? Visible warning to the user? I'm new to this so if I'm missing something, then some guidance would be appreciated. 👍 |
Why not compile class C {
static name = 'something';
static arguments = 'args';
static caller = 'caller';
}
console.log(C.name); // 'C'
console.log(C.arguments); // null
console.log(C.caller); // null to something like var C = (function () {
function C() {
}
C.__statics = {
name: 'something',
arguments: 'args',
caller: 'caller'
};
return C;
})();
console.log(C.__statics.name); // 'something'
console.log(C.__statics.arguments); // 'args'
console.log(C.__statics.caller); // 'caller' |
Question came up on stackoverflow as well : http://stackoverflow.com/a/34644236/390330 🌹 |
@zerkms please don't post the same comment in two places; it's confusing. You can't arbitrarily sometimes rewrite names in a structural type system. Consider some code like this interface HasName {
name: string;
}
class Foo {
static name = 'fooClass';
}
let bar: HasName = { name: string };
let q = Math.random() > 0.5 ? Foo : bar;
console.log(q.name); |
@RyanCavanaugh that's a fair example, indeed (I don't develop on TS, but gosh - its type system is weird as soon as it treats this code as valid). |
If it's not possible to transpile them without risk, perhaps we should just disallow them? |
Definitely not changing the transpile, just making it a compile error 🌹 |
Some enterprising developer should send us a PR! 😉 |
Why not use e.g. class C {
static length() { return "twelve"; }
} would be transplied to something like var C = (function () {
function C() {
}
Object.defineProperty(C, "length", {
value: function () { return "twelve"; },
writable: true
});
return C;
}()); |
@nicolo-ribaudo That does indeed work. My opinion: but I would rather see an error instead of such a transpile. TypeScript generally leans on the side of graceful error instead of fixing JavaScript 🌹 |
Is not the whole idea of TS is to fix the JS by filling the gaps it cannot and will never be able to. |
Yes. But by understanding how JavaScript works. Take an example of
|
Meanwhile ES6/2015 native support has landed in Chrome and Firefox. If we observe how these handle the issue natively then it comes close to what @nicolo-ribaudo and I have proposed (see #9778). Snippet to illustrate native behavior: class Foo {
constructor() {}
}
class Bar {
static length() {}
static name() {}
static caller() {}
}
Foo.name = "FooChanged";
Bar.name = "Baz";
console.log(Foo.name) // Logs "Foo". Foo.name remains unwritable
console.log(Bar.name) // Logs "Baz". Bar.name became writable With respect to what @tinganho has written earlier about the readonlyness of So using To sum things up, my take on the issue were:
Independent of the compile output I think any kind of additional message would be better than how the situation is today (TS 1.8.10) where we get no hint at all about potential problems with the compiler output. Edit: Snipped added |
@RyanCavanaugh: could you please revisit this issue for milestone TS 2.0.1. Solution seems straight forward but depending on which option you choose emitter output may need to change (see my previous comment). Release of TS 2.0 then may be best to include the fix. |
The tags on the PR indicate that the TypeScript team, which have limited resources, are leaving this open to the community to address. If you want this issue addressed, someone in the wider community will have to tackle it until the TypeScript core team feel they have sufficient room in the backlog to put it into a release (which likely would be a long long time). |
@kitsonk Thank you for explaining the PR label. I think I understood that. What I suggested was to reevaluate whether its still the right way to handle the issue. Static class properties called name or length are not so unlikely to happen and TS produces errorneous output for that. Hence I wouldn't even call the issue and "edge case". I consider static properties being at the core of the language and even if there aren't pull requests yet, solutions for the issue have been presented in this thread. Nevertheless, I will evaluate whether I can provide a PR but I must admit that I am not yet that knowledgeable about TS sources and I am afraid TS 2.0 will be released before. |
Fixing #442: Impossible to define static 'length' function on class
The following typescript program is accepted:
However, in the generated code:
Here, an attempt is made to assign to the
length
property of a function. This doesn't work (at least in Firefox and Chrome), causing subsequent calls ofC.length()
to crash. Perhaps calling a static class functionlength
should just be disallowed?Migrated from codeplex issue #1260.
The text was updated successfully, but these errors were encountered: