-
Notifications
You must be signed in to change notification settings - Fork 12.8k
do not error on 'super' property access in ES6 #5860
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
so how dose this work with non-accessor properties? |
@mhegazy ES6 natively support it so there is no error. However results might be quite unexpected. Consider this example: 'use strict'
class A {}
Object.defineProperty(A.prototype, "x", { value: 1 });
class A1 extends A {
method() { return super.x; }
}
console.log(new A1().x); // 1
console.log(new A1().method()); // 1
class B {
constructor() {
this.x = 2;
}
}
class B1 extends B {
method() { return super.x; }
}
console.log(new B1().x); // 2
console.log(new B1().method()); // undefined Reason is: |
yup. if you put it on the prototype it is going to work, except that all instances share the same variable, which is not expected, if you do not, super.x does not really point to any thing, as super and this are really the same object. that is the reason we did not allow it in the past. the question is should we do it now? we should talk about this in the design meeting. |
AFAIR main reason why we did not do this downlevel was: super access to data and accessor properties is different so we'll need to emit helper code to avoid runtime errors. For ES6 helpers are not necessary and the question is mostly: do we want to block potentially legitimate user scenarios just because we don't have way to know if super property access will use data or accessor property. |
pinging @RyanCavanaugh, can you pls confirm that this PR is approved on design meeting before I check it in? |
👍 |
do not error on 'super' property access in ES6
fixes #338. #4465