FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[Transforms] Down-level transformations for Async Functions by rbuckton · Pull Request #9175 · microsoft/TypeScript · GitHub

[Transforms] Down-level transformations for Async Functions - #9175

Merged
Ron Buckton (rbuckton) merged 31 commits into
transformsfrom
transforms-generators
Jul 20, 2016
Merged

[Transforms] Down-level transformations for Async Functions#9175
Ron Buckton (rbuckton) merged 31 commits into
transformsfrom
transforms-generators

Conversation

Copy link
Copy Markdown
Contributor

This change adds support for transforming a subset of generator function features to a down-level representation to support async functions when targeting ES5 or ES3.

Copy link
Copy Markdown
Contributor Author

Comment thread src/compiler/factory.ts
}

export function createTempVariable(recordTempVariable: (node: Identifier) => void, location?: TextRange): Identifier {
export function createTempVariable(recordTempVariable: ((node: Identifier) => void) | undefined, location?: TextRange): Identifier {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why not recordTempVariable?: (node: Identifier) => void?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

To catch mistakes. Its acceptable to not record the temp variable, but you almost always want to. The only cases where we don't are when we're creating a temp parameter, or we are going to add the temp variable name to a VariableDeclarationList ourselves.

Wesley Wigham (weswigham) commented Jun 17, 2016
edited
Loading

Copy link
Copy Markdown
Member

We need to handle directive prologues ("use strict";) when transforming async functions - I looked at the output of the tests/cases/conformance/async/es6/await[Binary,Call]Expression_es6 tests on this branch (which conveniently always have a string as their first statement!) and noticed that we move prologues into the generator function we synthesize - this is likely incorrect (even if it's our current behavior) if the consumer is using custom prologues (if they were just using "use strict" they may never notice). In any case, generators, also, need to special case retaining prologues as the first statement in the original outer function declaration (though, maybe not applicable right now since generators only arise as a consequence of async functions).

Also, would it be possible to just see the results of those same conformance tests running vs es5/es3? I know you added a bunch of new tests, but simply replicating the tests/cases/conformance/async/es6 directory for es5 and maybe es3 would be pertinent with this change. (And allow useful comparisons) #Resolved

Wesley Wigham (weswigham) commented Jun 17, 2016
edited
Loading

Copy link
Copy Markdown
Member

I don't think there's any issues, but I couldn't find a test to confirm - I don't see any tests (existing or otherwise) covering constructs such as class extends (await whatever) (which should be valid within an async function). Given we check our emit for await pretty much everywhere else, we should probably add tests in an extends clause.

define(["require", "exports"], function (require, exports) {
"use strict";
return (_a = {}, _a["hi"] = "there", _a);
return _a = {}, _a["hi"] = "there", _a;

Wesley Wigham (weswigham) Jun 17, 2016
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is the removal of the parenthesis in this test output and the one below intentional? #Resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes. The node factories will automatically add parenthesis if they are needed. As a result, many places where we explicitly added parentheses now instead leverage this behavior. In this instance, the createReturn factory does not need to parenthesize, so they are not added.

Copy link
Copy Markdown
Contributor Author

Yui (@yuit), Wesley Wigham (@weswigham): Please take another look following the recent commits.
Mohamed Hegazy (@mhegazy), Vladimir Matveev (@vladima), Daniel Rosenwasser (@DanielRosenwasser): Any comments?

Ron Buckton (rbuckton) merged commit d4ad7f3 into transforms Jul 20, 2016
Ron Buckton (rbuckton) deleted the transforms-generators branch July 20, 2016 19:12
Ron Buckton (rbuckton) restored the transforms-generators branch July 20, 2016 19:12

Copy link
Copy Markdown

Does this PR being merged mean we can use async/await now with transpilation to < ES6?

Copy link
Copy Markdown
Member

It's merged with the transforms branch, not master. It won't ship until transforms merges into master.

Wesley Wigham (weswigham) deleted the transforms-generators branch August 11, 2016 01:44

Copy link
Copy Markdown

Anything new on this one? When will it be available in typescript@next?

Copy link
Copy Markdown
Contributor

should start showing up in typescript@next next week.

Copy link
Copy Markdown

those using tslib and noEmitHelpers must install tslib from github as the npm package is not updated yet (and thus does not contain __generator).

Copy link
Copy Markdown

I got excited enough by @mghegazy's comment earlier that I went ahead and installed typescript@next on my project to start refactoring some of my more labyrinthine promise usage into async/await. And did not bother to check whether transforms had actually been merged into master 😄

And I was like, whoa, this already works when targeting ES5! But then I realized the output is still using ES6 generators, which just happen to work in Chrome and the iOS 10 webview I was testing in 🤐

Copy link
Copy Markdown
Contributor

And I was like, whoa, this already works when targeting ES5! But then I realized the output is still using ES6 generators, which just happen to work in Chrome and the iOS 10 webview I was testing in

jkobylec i do not think this is accurate. Emitting async functions for ES3/ES5 is supported in typescript@next.

here is a demonstration.

c:\test>tsc --v
Version 2.1.0-dev.20161014

c:\test>type a.ts
async function test() {
    await bar();
}

c:\test>tsc --target ES5 --lib es5,es2015.promise a.ts
a.ts(2,11): error TS2304: Cannot find name 'bar'.

c:\test>type a.js
var __awaiter =...
var __generator = ...

function test() {
    return __awaiter(this, void 0, void 0, function () {
        return __generator(this, function (_a) {
            switch (_a.label) {
                case 0: return [4 /*yield*/, bar()];
                case 1:
                    _a.sent();
                    return [2 /*return*/];
            }
        });
    });
}

Also i would recommend creating a new issue instead of commenting on a closed PR. thanks.

Copy link
Copy Markdown

My assumption was that the incorporation into master was still in progress
which is why I didn't open a new issue, but seems like the behavior I'm
encountering may be an oddity in my build tool chain I'll need to track
down, then. Apologies if I caused any alarm.
On Fri, Oct 14, 2016 at 5:35 PM Mohamed Hegazy notifications@github.com
wrote:

And I was like, whoa, this already works when targeting ES5! But then I
realized the output is still using ES6 generators, which just happen to
work in Chrome and the iOS 10 webview I was testing in

jkobylec https://github.com/jkobylec i do not think this is accurate.
Emitting async functions for ES3/ES5 is supported in typescript@next.

here is a demonstration.

c:\test>tsc --v
Version 2.1.0-dev.20161014

c:\test>type a.ts
async function test() {
await bar();
}

c:\test>tsc --target ES5 --lib es5,es2015.promise a.ts
a.ts(2,11): error TS2304: Cannot find name 'bar'.

c:\test>type a.js
var __awaiter =...
var __generator = ...

function test() {
return __awaiter(this, void 0, void 0, function () {
return __generator(this, function (_a) {
switch (_a.label) {
case 0: return [4 /yield/, bar()];
case 1:
_a.sent();
return [2 /return/];
}
});
});
}

Also i would recommend creating a new issue instead of commenting on a
closed PR. thanks.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9175 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHtc0jzelV11ieHBGLK28go-4apOf1c1ks5q0B-8gaJpZM4I16H6
.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Domain: API: Transforms Relates to the public transform API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants


Back | FazBrowse Home | New Git URL