Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
rbuckton merged 31 commits intotransformsfromtransforms-generators
Jul 20, 2016

Conversation

@rbuckton
Copy link
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.

arciisine, chicoxyzzy, WanderWang, aharpervc, eliashdezr, abierbaum, mike-morr, callumlocke, hugues-m, jnm2, and 35 more reacted with thumbs up emojiDickvdBrink, JeroenVinke, jjwon0, jwbay, HerringtonDarkholme, Luanre, gulbanana, aharpervc, tinganho, johnfn, and 63 more reacted with hooray emoji
@rbucktonrbuckton added the Domain: API: TransformsRelates to the public transform API labelJun 15, 2016
@rbuckton
Copy link
ContributorAuthor

}

exportfunctioncreateTempVariable(recordTempVariable:(node:Identifier)=>void,location?:TextRange):Identifier{
exportfunctioncreateTempVariable(recordTempVariable:((node:Identifier)=>void)|undefined,location?:TextRange):Identifier{
Copy link
Member

Choose a reason for hiding this comment

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

Why notrecordTempVariable?: (node: Identifier) => void?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

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 aVariableDeclarationList ourselves.

@weswigham
Copy link
Member

weswigham commentedJun 17, 2016
edited
Loading

We need to handle directive prologues ("use strict";) when transforming async functions - I looked at the output of thetests/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 thetests/cases/conformance/async/es6 directory fores5 and maybees3 would be pertinent with this change. (And allow useful comparisons) #Resolved

rbuckton reacted with thumbs up emoji

@weswigham
Copy link
Member

weswigham commentedJun 17, 2016
edited
Loading

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 asclass extends (await whatever) (which should be valid within anasync function). Given we check our emit for await pretty much everywhere else, we should probably add tests in an extends clause.

rbuckton reacted with thumbs up emoji

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

@weswighamweswighamJun 17, 2016
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

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, thecreateReturn factory does not need to parenthesize, so they are not added.

@rbuckton
Copy link
ContributorAuthor

@yuit,@weswigham: Please take another look following the recent commits.
@mhegazy,@vladima,@DanielRosenwasser: Any comments?

@rbucktonrbuckton merged commitd4ad7f3 intotransformsJul 20, 2016
@rbucktonrbuckton deleted the transforms-generators branchJuly 20, 2016 19:12
@rbucktonrbuckton restored the transforms-generators branchJuly 20, 2016 19:12
@olee
Copy link

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

@sandersn
Copy link
Member

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

@weswighamweswigham deleted the transforms-generators branchAugust 11, 2016 01:44
@cime
Copy link

cime commentedSep 1, 2016

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

normalser reacted with thumbs up emoji

@mhegazy
Copy link
Contributor

should start showing up intypescript@next next week.

buzinas, arciisine, devzsolt, trotyl, cedricr, olee, Glavin001, jichang, 0xphilipp, frankmarineau, and 4 more reacted with thumbs up emojinormalser, glen-84, masterpoi, vilicvane, jonaskello, JeroenVinke, pleerock, jocull, buzinas, drywolf, and 18 more reacted with hooray emojipiotrwitek, JeroenVinke, buzinas, arciisine, tedvanderveen, trotyl, olee, Glavin001, kenjiru, cebor, and 2 more reacted with heart emoji

@nippur72
Copy link

those usingtslib andnoEmitHelpers must installtslib from github as the npm package is not updated yet (and thus does not contain__generator).

atsu85 referenced this pull request in aurelia/template-lintOct 9, 2016
@jkobylec
Copy link

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 whethertransforms 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 🤐

@mhegazy
Copy link
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 intypescript@next.

here is a demonstration.

c:\test>tsc --vVersion 2.1.0-dev.20161014c:\test>type a.tsasync function test() {    await bar();}c:\test>tsc --target ES5 --lib es5,es2015.promise a.tsa.ts(2,11): error TS2304: Cannotfind name 'bar'.c:\test>type a.jsvar __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.

@jkobylec
Copy link

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 Hegazynotifications@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

@jkobylechttps://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
.

@microsoftmicrosoft locked and limited conversation to collaboratorsJun 19, 2018
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Domain: API: TransformsRelates to the public transform API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

11 participants

@rbuckton@weswigham@olee@sandersn@cime@mhegazy@nippur72@jkobylec@yuit@msftclas

[8]ページ先頭

©2009-2025 Movatter.jp