Skip to content

Update async await transpilation to avoid captures of super #3103

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Update async await transpilation to avoid captures of super #3103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56401a1
Update async await transpilation to avoid captures of `super` when us…
ChadKillingsworth Oct 12, 2018
cfa4af4
Simpler super alternatives in static member methods for async transpi…
ChadKillingsworth Oct 15, 2018
20b4c32
Call Object.getPrototypeOf twice to avoid referencing this.constructor
ChadKillingsworth Oct 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions src/com/google/javascript/jscomp/RewriteAsyncFunctions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,33 @@ private void convertAsyncFunction(NodeTraversal t, LexicalContext functionContex
newBody.addChildToBack(IR.constNode(IR.name(ASYNC_ARGUMENTS), IR.name("arguments")));
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.CONST_DECLARATIONS);
}
boolean needsSuperTranspilation = compiler.getOptions().needsTranspilationFrom(FeatureSet.ES6);
for (String replacedMethodName : functionContext.replacedSuperProperties) {
// MS Edge 17 cannot properly capture references to "super" in an arrow function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any link we could insert here pointing to the MS Edge bug?
We'd really like to be able to track when the MS Edge bug gets fixed, or if it's already fixed and we're just stuck because of installed browsers not getting the fix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created one and added the link

// See https://github.com/Microsoft/ChakraCore/issues/5784
// If we are not transpiling classes, switch to using Object.getPrototypeOf(this)
// as a replacement for super.
// If we are transpiling classes, the super reference will be handled elsewhere.
Node superReference;
if (needsSuperTranspilation) {
superReference = IR.superNode();
} else {
// static super: Object.getPrototypeOf(this);
superReference =
IR.call(IR.getprop(IR.name("Object"), IR.string("getPrototypeOf")), IR.thisNode());
if (!originalFunction.getParent().isStaticMember()) {
// instance super: Object.getPrototypeOf(Object.getPrototypeOf(this))
superReference =
IR.call(IR.getprop(IR.name("Object"), IR.string("getPrototypeOf")), superReference);
}
}

// const super$get$x = () => super.x;
Node arrowFunction = IR.arrowFunction(
IR.name(""), IR.paramList(), IR.getprop(IR.superNode(), IR.string(replacedMethodName)));
Node arrowFunction =
IR.arrowFunction(
IR.name(""),
IR.paramList(),
IR.getprop(superReference, IR.string(replacedMethodName)));
compiler.reportChangeToChangeScope(arrowFunction);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.ARROW_FUNCTIONS);

Expand Down
23 changes: 22 additions & 1 deletion src/com/google/javascript/jscomp/RewriteAsyncIteration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,17 +557,38 @@ private void prependTempVarDeclarations(LexicalContext ctx, NodeTraversal t) {
IR.constNode(IR.name(argumentsVarName), IR.name("arguments"))
.useSourceInfoFromForTree(block));
}
boolean needsSuperTranspilation = compiler.getOptions().needsTranspilationFrom(FeatureSet.ES6);
for (String replacedMethodName : thisSuperArgsCtx.usedSuperProperties) {
// { // prefixBlock
// const $jscomp$asyncIter$this = this;
// const $jscomp$asyncIter$arguments = arguments;
// const $jscomp$asyncIter$super$get$x = () => super.x;
// }
//
// MS Edge 17 cannot properly capture references to "super" in an arrow function.
// See https://github.com/Microsoft/ChakraCore/issues/5784
// If we are not transpiling classes, switch to using Object.getPrototypeOf(this)
// as a replacement for super.
// If we are transpiling classes, the super reference will be handled elsewhere.
Node superReference;
if (needsSuperTranspilation) {
superReference = IR.superNode();
} else {
// static super: Object.getPrototypeOf(this);
superReference =
IR.call(IR.getprop(IR.name("Object"), IR.string("getPrototypeOf")), IR.thisNode());
if (!ctx.function.getParent().isStaticMember()) {
// instance super: Object.getPrototypeOf(Object.getPrototypeOf(this))
superReference =
IR.call(IR.getprop(IR.name("Object"), IR.string("getPrototypeOf")), superReference);
}
}

Node arrowFunction =
IR.arrowFunction(
IR.name(""),
IR.paramList(),
IR.getprop(IR.superNode(), IR.string(replacedMethodName)));
IR.getprop(superReference, IR.string(replacedMethodName)));
compiler.reportChangeToChangeScope(arrowFunction);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.ARROW_FUNCTIONS);
String superReplacementName = superPropGetterPrefix + replacedMethodName;
Expand Down
6 changes: 4 additions & 2 deletions test/com/google/javascript/jscomp/IntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3988,7 +3988,8 @@ public void testAsyncFunctionSuper() {
"}",
"class Baz extends Foo {",
" bar() {",
" const $jscomp$async$this = this, $jscomp$async$super$get$bar = () => super.bar;",
" const $jscomp$async$this = this, $jscomp$async$super$get$bar =",
" () => Object.getPrototypeOf(Object.getPrototypeOf(this)).bar;",
" return $jscomp.asyncExecutePromiseGeneratorFunction(function*() {",
" yield Promise.resolve();",
" $jscomp$async$super$get$bar().call($jscomp$async$this);",
Expand Down Expand Up @@ -4041,7 +4042,8 @@ public void testAsyncIterationSuper() {
"class Baz extends Foo {",
" bar() {",
" const $jscomp$asyncIter$this = this,",
" $jscomp$asyncIter$super$get$bar = () => super.bar;",
" $jscomp$asyncIter$super$get$bar =",
" () => Object.getPrototypeOf(Object.getPrototypeOf(this)).bar;",
" return new $jscomp.AsyncGeneratorWrapper(function*() {",
" $jscomp$asyncIter$super$get$bar().call($jscomp$asyncIter$this).next();",
" }());",
Expand Down
67 changes: 67 additions & 0 deletions test/com/google/javascript/jscomp/RewriteAsyncFunctionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,73 @@ public void testInnerSuperReference() {
"}"));
}

@Test
public void testInnerSuperCallEs2015Out() {
setLanguageOut(LanguageMode.ECMASCRIPT_2015);
test(
lines(
"class A {",
" m() {",
" return this;",
" }",
"}",
"class X extends A {",
" async m() {",
" return super.m();",
" }",
"}"),
lines(
"class A {",
" m() {",
" return this;",
" }",
"}",
"class X extends A {",
" m() {",
" const $jscomp$async$this = this;",
" const $jscomp$async$super$get$m =",
" () => Object.getPrototypeOf(Object.getPrototypeOf(this)).m;",
" return $jscomp.asyncExecutePromiseGeneratorFunction(",
" function* () {",
" return $jscomp$async$super$get$m().call($jscomp$async$this);",
" });",
" }",
"}"));
}

@Test
public void testInnerSuperCallStaticEs2015Out() {
setLanguageOut(LanguageMode.ECMASCRIPT_2015);
test(
lines(
"class A {",
" static m() {",
" return this;",
" }",
"}",
"class X extends A {",
" static async m() {",
" return super.m();",
" }",
"}"),
lines(
"class A {",
" static m() {",
" return this;",
" }",
"}",
"class X extends A {",
" static m() {",
" const $jscomp$async$this = this;",
" const $jscomp$async$super$get$m = () => Object.getPrototypeOf(this).m;",
" return $jscomp.asyncExecutePromiseGeneratorFunction(",
" function* () {",
" return $jscomp$async$super$get$m().call($jscomp$async$this);",
" });",
" }",
"}"));
}

@Test
public void testNestedArrowFunctionUsingThis() {
test(
Expand Down
3 changes: 2 additions & 1 deletion test/com/google/javascript/jscomp/RewriteAsyncIterationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ public void testInnerSuperReferenceInAsyncGenerator() {
"}",
"class X extends A {",
" m() {",
" const $jscomp$asyncIter$super$get$m = () => super.m;",
" const $jscomp$asyncIter$super$get$m =",
" () => Object.getPrototypeOf(Object.getPrototypeOf(this)).m;",
" return new $jscomp.AsyncGeneratorWrapper(",
" function* () {",
" const tmp = $jscomp$asyncIter$super$get$m();",
Expand Down