Skip to content
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

Arrow functions of a class show up on prototype #20922

Closed
moraneva opened this issue Dec 28, 2017 · 6 comments
Closed

Arrow functions of a class show up on prototype #20922

moraneva opened this issue Dec 28, 2017 · 6 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@moraneva
Copy link

moraneva commented Dec 28, 2017

TypeScript Version: 2.7.0-dev.20171226

Code

class Test {
    constructor() { }

    public testPrototypeMethod() {

    }

    public testArrowMethod = () => { }
}

console.log(Test.prototype.testArrowMethod); // This should not compile
console.log(Test.prototype.testPrototypeMethod);

Expected behavior:
Test.prototype.testArrowMethod should fail on compilation because it isn't on the classes prototype.

Actual behavior:
Test.prototype.testArrowMethod does not fail on compilation but becomes undefined at runtime.

Reason
The code above compiles into the following js:

var Test = /** @class */ (function () {
    function Test() {
        this.testArrowMethod = function () { };
    }
    Test.prototype.testPrototypeMethod = function () {
    };
    return Test;
}());

As can be seen, the arrow function is actually defined using this instead of prototype on instantiation. This is fine, but testArrowMethod should not be able to be accessed from Test.prototype

@poseidonCore
Copy link

What you describing is expected behaviour for any initialised public property of a class (not just arrow functions).

Whether Test.prototype.testArrowMethod should be picked up as an error is something that a language developer will have to comment on, but it is expected to be undefined until the class is instantiated.

As far as I know, this is due to a deep language-design decision that revolves around an expectation that initialised class properties be owned by the instance, whereas class methods fall through to the prototype.

Traditionally, when rolling your own JS-style classes, you could choose between setting a default value of a property the long-hand way that TS compiles to, or by setting the prototype directly: eg

// Straight JS code:

function Test() {}
Test.prototype.testArrowMethod = function () { };

var test = new Test();

test.x = 1;

If you do it this way, then the property is immediately available on the prototype, rather than having to wait for instantiation, but this leads to an inconsistency with properties that are later set (eg x) because here testArrowMethod is not owned by the instance, whereas x is.

You can see the effect on your own code as follows:

class Test {
    constructor() { }

    public testPrototypeMethod() {}

    public testArrowMethod = () => {}

    public x = 1;
}

// @ts-ignore
Test.prototype.y = 2;

var test = new Test();

console.log(test.hasOwnProperty("testPrototypeMethod")); // false
console.log(test.hasOwnProperty("testArrowMethod")); // true
console.log(test.hasOwnProperty("x")); // true
console.log(test.hasOwnProperty("y")); // false

console.log(Test.prototype.x); // undefined
// @ts-ignore
console.log(Test.prototype.y); // 2

When you roll your own code, then that design decision is yours, but I believe that this decision was made to also accommodate some constraints placed upon the system by the way that super works when extending classes.

Perhaps a language developer can clarify this.

@moraneva
Copy link
Author

@poseidonCore Yep you are right, I didn't realize that it is that way for any initialized property of a class. I'm just not sure if there is a reason why those properties would show up on the classes prototype when using Typescript. It seems to me like this shouldn't happen, but maybe a language dev can comment more on that.

@RyanCavanaugh
Copy link
Member

The prototype property has the same type as the instance type of the class; we don't do the necessary work to determine if the property is actually present since the only normal use of .prototype is for something like className.prototype.method.call(, etc..

In theory we could synthesize the "correct" type but it doesn't seem to matter in practice, and might break other stuff since this has been the behavior since first release. Did this behavior manifest as a bug in your code, or did you just happen to notice it?

@moraneva
Copy link
Author

moraneva commented Dec 29, 2017

@RyanCavanaugh that makes sense. It's just confusing when using type hinting and things comes up when they aren't actually present on prototype.

I happened to notice this when I was attempting to use jest to mock an arrow function on a class:

Test.prototype.testArrowMethod = jest.fn()....

I was also having issues trying to spy on that function, which ended up being the exact same issue (testArrowMethod not actually existing on .prototype)

So the solution for me was just to make it a non arrow-function.

@poseidonCore
Copy link

Also, bear in mind that TS (unlike some other languages) allows initialisation to non-primitive values: eg

class Person {
   birthDate = new Date(0);
}

and by initialising this in the constructor (rather than the prototype) means that these non-primitive objects are not shared between instances, and so person1.birthDate.setFullYear() should not affect person2.birthDate, ... whereas if you initialise these in the prototype, then they are linked in the prototype and affect all instances that have not overridden that property directly.

class Person {
   birthDate:Date;
}

Person.prototype.birthDate = new Date(0);

var person1 = new Person();
var person2 = new Person();

console.log(person1.birthDate.getFullYear()); // 1970
console.log(person2.birthDate.getFullYear()); // 1970

person1.birthDate.setFullYear(2018);

console.log(person1.birthDate.getFullYear()); // 2018
console.log(person2.birthDate.getFullYear()); // 2018

Compare that to standard TS:

class Person {
   birthDate = new Date(0);
}

var person1 = new Person();
var person2 = new Person();

console.log(person1.birthDate.getFullYear()); // 1970
console.log(person2.birthDate.getFullYear()); // 1970

person1.birthDate.setFullYear(2018);

console.log(person1.birthDate.getFullYear()); // 2018
console.log(person2.birthDate.getFullYear()); // 1970

This relates to the importance of instances owning their own properties in this regard and applies a standard approach. If the initialisation behaviour was changed to put the initial value in the prototype, this would affect any code that did things as noted here.

@RyanCavanaugh RyanCavanaugh added Declined The issue was declined as something which matches the TypeScript vision and removed In Discussion Not yet reached consensus labels Feb 6, 2018
@RyanCavanaugh
Copy link
Member

This would be a big breaking change with very little upside in terms of actually catching bugs you would plausibly write.

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants