GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [Transforms] Down-level transformations for Async Functions by rbuckton · Pull Request #9175 · microsoft/TypeScript

Conversation

@rbuckton

@rbuckton Ron Buckton (rbuckton) commented Jun 15, 2016

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

arciisine, chicoxyzzy, WanderWang, aharpervc, eliashdezr, abierbaum, mizzle-mo, callumlocke, hugues-m, jnm2, and 35 more reacted with thumbs up emoji DickvdBrink, JeroenVinke, jjwon0, jwbay, HerringtonDarkholme, Luanre, gulbanana, aharpervc, tinganho, johnfn, and 63 more reacted with hooray emoji
All reactions
  • 👍 45 reactions
  • 🎉 73 reactions
Ron Buckton (rbuckton) added 15 commits February 29, 2016 11:04
@rbuckton
Early support for generators.
d506e92
@rbuckton
Merge branch 'transforms-transformer-module' into transforms-transfor…
88b38f8
…mer-es6-generators
@rbuckton
Merge branch 'transforms-transformer-module' into transforms-transfor…
aaf3ab7
…mer-es6-generators
@rbuckton
Fixed labels
c267691
@rbuckton
merge branch 'transforms-fixMoreSourceMaps' into transforms-transform…
83b4cbb
…er-es6-generators
@rbuckton
Fix issues after merge
4ae83fa
@rbuckton
Merge branch 'transforms' into transforms-generators
e85d3e6
@rbuckton
Cleanup, only support generators for async functions, added emit helper.
0948f73
@rbuckton
Adds progress indicators to the runtests-parallel build task.
8190666
@rbuckton
Fixed typo
3681e3c
@rbuckton
Fixes an issue with runtests-parallel when global mocha is not instal…
4afb8c4
…led.
@rbuckton
Fixes runtests-parallel not reporting failure for failed tests.
aa4662e
@rbuckton
Cleanup, fix linter errors, follow hoisted declarations.
27931d5
@rbuckton
Added tests and fixed related emit issues.
ea61f2b
@rbuckton
Merge branch 'transforms' into transforms-generators
9073572
@rbuckton Ron Buckton (rbuckton) added the Domain: API: Transforms Relates to the public transform API label Jun 15, 2016
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Jun 15, 2016
Ron Buckton (rbuckton) added 3 commits June 15, 2016 11:04
@rbuckton
Merge branch 'transforms' into transforms-generators
5e31b25
@rbuckton
Merge branch 'transforms' into transforms-generators
4eb2a82
@rbuckton
Add support for external helpers module, cleanup, and accept new base…
278a350
…lines.
@rbuckton

Ron Buckton (rbuckton) commented Jun 16, 2016

Copy link
Copy Markdown
Contributor Author

CC: Vladimir Matveev (@vladima), Mohamed Hegazy (@mhegazy), Yui (@yuit), Nathan Shively-Sanders (@sandersn), Wesley Wigham (@weswigham), Daniel Rosenwasser (@DanielRosenwasser)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

weswigham
weswigham reviewed Jun 17, 2016
View reviewed changes
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 {

@weswigham Wesley Wigham (weswigham) Jun 17, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@rbuckton Ron Buckton (rbuckton) Jun 17, 2016

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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 a VariableDeclarationList ourselves.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@weswigham

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

Uh oh!

There was an error while loading. Please reload this page.

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

rbuckton reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@weswigham

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

Uh oh!

There was an error while loading. Please reload this page.

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.

rbuckton reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

weswigham
weswigham reviewed Jun 17, 2016
View reviewed changes
Comment thread tests/baselines/reference/exportEqualsAmd.js
define(["require", "exports"], function (require, exports) {
"use strict";
return (_a = {}, _a["hi"] = "there", _a);
return _a = {}, _a["hi"] = "there", _a;

@weswigham Wesley Wigham (weswigham) Jun 17, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@rbuckton Ron Buckton (rbuckton) Jun 22, 2016

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Ron Buckton (rbuckton) added 7 commits June 27, 2016 13:48
@rbuckton
Move 'use strict' directive out of generator for async function.
393ee28
@rbuckton
Added es5 conformance tests for await in binary.
5b2e11c
@rbuckton
Added es5 conformance tests for async function declarations. Add abil…
48a9562
…ity to treat some statements as if they were prologue directives.
@rbuckton
Added es5 conformance tests for awaiting a call
0c647c3
@rbuckton
Added additional es5 conformance tests, better emit for functions ret…
203dab4
…urning global promise, pass this to __generator
@rbuckton
Added es5 conformance tests for async arrow functions, add error for …
644e4da
…referencing 'arguments' in a generator.
@rbuckton
Additional comments per PR feedback.
9e3d6f8
@rbuckton

Ron Buckton (rbuckton) commented Jun 28, 2016

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MattiasBuelens Mattias Buelens (MattiasBuelens) mentioned this pull request Jul 1, 2016
ES6 Generator support #1564
Closed
@frederickfogerty Frederick Fogerty (frederickfogerty) mentioned this pull request Jul 17, 2016
Move global DSL type definitions into an index file. philcockfield/ui-harness#93
Closed
Ron Buckton (rbuckton) added 3 commits July 18, 2016 15:43
@rbuckton
Merge branch 'transforms' into transforms-generators
252cc25
@rbuckton
Accept baselines plus new linter warnings.
c832444
@rbuckton
Updated comment
c285767
@rbuckton
Ron Buckton (rbuckton) merged commit d4ad7f3 into transforms Jul 20, 2016
@rbuckton
Ron Buckton (rbuckton) deleted the transforms-generators branch July 20, 2016 19:12
@rbuckton
Ron Buckton (rbuckton) restored the transforms-generators branch July 20, 2016 19:12
@olee

Björn Zeutzheim (olee) commented Jul 22, 2016

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn

Nathan Shively-Sanders (sandersn) commented Jul 22, 2016

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Andrej Cimperšek (cime) commented Sep 1, 2016

Copy link
Copy Markdown

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

normalser reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhegazy

Mohamed Hegazy (mhegazy) commented Sep 1, 2016

Copy link
Copy Markdown
Contributor

should start showing up in typescript@next next week.

buzinas, arciisine, devzsolt, trotyl, cedricr, olee, Glavin001, jichang, 0xphilipp, frankmarineau, and 4 more reacted with thumbs up emoji normalser, glen-84, masterpoi, vilicvane, jonaskello, JeroenVinke, pleerock, jocull, buzinas, drywolf, and 18 more reacted with hooray emoji piotrwitek, JeroenVinke, buzinas, arciisine, tedvanderveen, trotyl, olee, Glavin001, kenjiru, cebor, and 2 more reacted with heart emoji
All reactions
  • 👍 14 reactions
  • 🎉 28 reactions
  • ❤️ 12 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nippur72

nino-porcino (nippur72) commented Sep 18, 2016

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).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tony19 Tony Trinh (tony19) mentioned this pull request Oct 2, 2016
Add support for http2 Polymer/polyserve#98
Merged
Ats Uiboupin (atsu85) referenced this pull request in aurelia/template-lint Oct 9, 2016
@MeirionHughes
feat(rework): new layout
a59b362
@jkobylec

jkobylec commented Oct 14, 2016

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 🤐

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhegazy

Mohamed Hegazy (mhegazy) commented Oct 15, 2016

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jkobylec

jkobylec commented Oct 15, 2016

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 [email protected]
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
.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@DickvdBrink Dick van den Brink (DickvdBrink) mentioned this pull request Oct 19, 2016
Support async/await in ES5 #5211
Closed
@kitsonk Kitson Kelly (kitsonk) mentioned this pull request Nov 15, 2016
Migrate to TypeScript 2.1 dojo/meta#111
Closed
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Domain: API: Transforms Relates to the public transform API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

@rbuckton @weswigham @olee @sandersn @cime @mhegazy @nippur72 @jkobylec @yuit @msftclas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.