Add transpilation support for properties and computed properties from super#2418
Add transpilation support for properties and computed properties from super#2418ChadKillingsworth wants to merge 4 commits intogoogle:masterfrom
Conversation
| } | ||
| }); | ||
|
|
||
| if (exprRoot.getParent().isNew()) { |
There was a problem hiding this comment.
Please add code examples to make it clearer what each of these cases represent.
e.g.
if (exprRoot.getParent().isNew()) {
// new super()
| case MEMBER_FUNCTION_DEF: | ||
| case GETTER_DEF: | ||
| case SETTER_DEF: | ||
| case COMPUTED_PROP: |
There was a problem hiding this comment.
Why add COMPUTED_PROP? Are you trying to support something like this?
class Foo extends Bar {
[super.something]() {}
}There was a problem hiding this comment.
No - need to recognize access to super within a computed property function:
class Foo extends Bar {
[someThing]() {
super.otherThing();
}
}There was a problem hiding this comment.
I believe it was this test case: https://github.com/google/closure-compiler/pull/2418/files#diff-8cd1c911f8bd71b40937a5cdb2dbca85R1025
| potentialCallee.replaceChild(node, superName.cloneTree()); | ||
| } else if (enclosingMemberDef.isStaticMember()) { | ||
| if (callTarget.isSuper()) { | ||
| compiler.report(JSError.make(node, INVALID_SUPER)); |
There was a problem hiding this comment.
Before doing this PR, I think we'd better have one that removes support for calling super() in a non-constructor method.
| for (; | ||
| exprRoot.getParent().isGetElem() || exprRoot.getParent().isGetProp(); | ||
| exprRoot = exprRoot.getParent()) { | ||
| if (exprRoot.getParent().getFirstChild() != exprRoot) { |
There was a problem hiding this comment.
I think this is going too far.
If you have super.foo.bar.baz(), we only need to go as far as super.foo to determine what to do.
|
This should be unblocked now that #2452 is closed. |
03ad39b to
44720b2
Compare
|
@brad4d ready for review again. |
44720b2 to
bfb9f14
Compare
|
The Travis failure was probably because our build was broken over the weekend. |
|
You can trigger travis to rerun by closing then re-opening a PR. |
|
importing and testing... |
|
primary testing looks good |
|
Tests looking good. |
|
Adding tests for access to a property defined with a getter and a setter before submitting. |
|
@ChadKillingsworth |
|
@ChadKillingsworth |
|
I would lean toward |
|
I've got code that depends on transpiling super properties - and polymer uses it. It's not that hard to fix, but it'll probably need a runtime helper injected. I'll work on it this week. |
|
Disallowed assigning to a super property. @brad4d I assume you'll need to fix the runtime test. Since those don't run externally, I would just be guessing. For posterity, here's how Babel transpiles assigning to a super property: var _set = function set(object, property, value, receiver) {
var desc = Object.getOwnPropertyDescriptor(object, property);
if (desc === undefined) {
var parent = Object.getPrototypeOf(object);
if (parent !== null) {
set(parent, property, value, receiver);
}
} else if ("value" in desc && desc.writable) {
desc.value = value;
} else {
var setter = desc.set;
if (setter !== undefined) {
setter.call(receiver, value);
}
}
return value;
};
Foobar.prototype.someMethod() {
_set(Foobar.prototype.__proto__ || Object.getPrototypeOf(Foobar.prototype),
"someprop", "someval", this);
} |
0506050 to
a256afc
Compare
|
Imported. |
|
submitted internally |
…roperties Closes google#2418 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=158983919
This adds support for property references off of super which has been missing from our transpilation support.