Support Async/Await based on #12 #125

shirotech · 2018-04-10T22:46:25Z

It's been a very long time since there has been activity to this pull request #12 I have cleanly integrated the code using latest master with minor improvements and seems to pass all tests. Hope this is appreciated and merged to codebase as I believe many have requested this feature.

Closes #90, Closes #108, Closes #109, Closes #113

shirotech · 2018-04-12T18:27:55Z

Now that I think about it, should it be the responsible of buble to transpile async code by default? I’m happy to introduce a “dangerous” flag for it? What do you guys think?

aleclarson · 2018-05-14T12:20:57Z

Need some tests for:

const res = await promise;
return (await promise).prop;
let val = foo(); val = await bar(val); val = mux(val + 1);

Also, in this test case, could we get thing() and stuff() calls into the same function as the await statements after each of them?

aleclarson · 2018-05-14T13:45:15Z

test/samples/async-await.js

+	{
+		description: 'transpiles await arrow function call',
+		input: `async () => await a()`,
+		output: `!function() { return Promise.resolve().then(function() { return a() }); }`


What's the point of !function? Just to draw attention that it's not being used?

It turns it from a function declaration into an expression, see: https://stackoverflow.com/questions/3755606/what-does-the-exclamation-mark-do-before-the-function

As for why I'm not entirely sure.

The input is a function expression, too.

mrTimofey · 2018-08-07T11:46:23Z

Is this going to be released some day? It's kinda good stuff but seems like no one is going to merge and release it...

aleclarson · 2018-08-31T21:38:23Z

@mrTimofey Lots of edge cases are unaccounted for in this PR, and the author is MIA.

KutnerUri · 2018-10-14T08:40:05Z

please add test making sure this works:
const { a, b } = await abFunc();

this doesn't work correctly in Bable, and it caused some nasty bugs for us.

StephanHoyer · 2018-10-26T11:38:21Z

test/samples/async-await.js

+		description: 'transpiles await function call with more than one line of code',
+		input: `async function f() { await a(); thing(); await a2(); stuff(); await a3(); await a4(); }`,
+		output: `function f() { return Promise.resolve().then(function() { return a(); }) .then(function() { thing(); }) .then(function() { return a2(); }) .then(function() { stuff(); }) .then(function() { return a3(); }) .then(function() { return a4(); }).then(function() {}) }`
+	}


there is not test that verifies return values and closures

{ description: 'transpiles await function call with normal return statement', input: `async function f() { var x = 1; await a(); return x; }`, output: `...` },

shirotech · 2018-10-31T21:32:06Z

Sorry guys seems this PR is far from perfect, I have only taken it from an existing PR. Will try my best to resolve.

adrianheine · 2018-11-01T19:28:47Z

src/support.js

 	}
 };

 export const features = [
 	'arrow',
+	'asyncAwait',
 	'classes',
 	'computedProperty',
 	'conciseMethodProperty',


This whole file can stay as it is, since I already added asyncAwait.

adrianheine · 2018-11-01T19:29:08Z

src/index.js

@@ -15,8 +15,8 @@ const dangerousTransforms = ['dangerousTaggedTemplateString', 'dangerousForOf'];
 export function target(target) {
 	const targets = Object.keys(target);
 	let bitmask = targets.length
-		? 0b11111111111111111111
-		: 0b01000000000000000000;


This change is not necessary anymore.

adrianheine · 2018-11-01T19:31:22Z

.eslintrc

@@ -5,6 +5,7 @@
        "semi": [ 2, "always" ],
        "keyword-spacing": [ 2, { "before": true, "after": true } ],
        "space-before-blocks": [ 2, "always" ],
+        "space-in-parens": [ 2, "never" ],
        "no-mixed-spaces-and-tabs": [ 2, "smart-tabs" ],
        "no-cond-assign": [ 0 ]
    },


This should be a separate change.

curran

Needs more work here but excited to see the feature land some day!

curran · 2018-11-13T06:08:08Z

src/program/BlockStatement.js

+			const last = this.body[this.body.length - 1];
+			const hasOnlyOneLine = this.body.length === 1;
+
+			// TODO refactor :)


Refactor how? Who wrote this comment?

Better refactor now before merging, as later it will most likely not ever be refactored.

curran · 2018-11-13T06:09:16Z

src/program/BlockStatement.js

+				}
+
+			} else if (this.parent.type === 'ArrowFunctionExpression') {
+				// TODO merge with ^


TODOs should be new issues IMO, and not included in the code.

curran · 2018-11-13T06:09:56Z

src/program/types/AwaitExpression.js

+export default class AwaitExpression extends Node {
+	static removeAsync(code, transforms, async, start, callback = noop) {
+		if (transforms.asyncAwait && async) {
+			code.remove(start, start + 6);


Magic number 6 is mysterious here. Better as named variable, or with a comment?

could this be "async ".length?

dy · 2018-11-15T07:05:10Z

Is this ever going to be merged?

curran · 2018-11-15T11:41:57Z

@dy It looks like the author of this PR @Van-Nguyen is not able to put in more time here and wrap it up. Someone else probably needs to make a new PR based on this one, and clean it up. I'm not sure who.

adrianheine · 2018-11-15T20:05:45Z

I've rebased the commit and already merged the unrelated linting part as acd791f.

adrianheine · 2018-11-15T20:37:57Z

src/program/BlockStatement.js

+			const hasOnlyOneLine = this.body.length === 1;
+
+			// TODO refactor :)
+			if (this.parent.type === 'FunctionDeclaration') {


This does not handle FunctionExpressions.

adrianheine · 2018-11-15T20:39:05Z

src/program/BlockStatement.js

+			if (this.parent.type === 'FunctionDeclaration') {
+				if (hasOnlyOneLine) {
+					if (first.type === 'ReturnStatement') {
+						code.insertLeft(first.argument.start, 'Promise.resolve().then(function() { ');


insertLeft has been replaced by appendLeft.

adrianheine · 2018-11-15T20:40:22Z

src/program/types/AwaitExpression.js

+
+	transpile (code, transforms) {
+		AwaitExpression.removeAsync(code, transforms, true, this.start, () => {
+			code.insertLeft(this.argument.start, 'return ');


This handles the expression as though it was a statement. See for example async () => await 1 + await 2.

adrianheine · 2018-11-16T16:46:36Z

@KutnerUri @aleclarson Could you open pull requests adding tests for the cases you mentioned to test/samples/async.js? I'd like to see this move forward at least a bit.

KutnerUri · 2018-11-16T17:35:47Z

@adrianheine - ok!
this is actually a great way for me to get started.
Don't you mean adding the tests at test/samples/async-await.js ? it seems more suited.

thinking about the tests, I think variables should be hoisted to the top of the function, otherwise it is going to be very difficult to transpile.

I think we need to add 'run code' tests, with expected results instead of expected transpiled.
This would be more abstract on how the code is transpiled.

adrianheine · 2018-11-16T17:47:03Z

@KutnerUri No, there is already a file test/samples/async.js in master, that's where these tests should land. As for tests that execute code, I'm already doing this with test262, see scripts/run_test262.js. I think we need unit tests that check the exact code produced, but I understand that they are difficult to write before the feature itself is implemented. Maybe you start with rather simple cases where it's obvious what the resulting code should look like?

KutnerUri · 2018-11-16T18:28:38Z

yes, tests on the produced code are a must, but the code should also pass correctness tests.
I am already writing my use case.

so should I just replace errors in async.js with the expected produced code?

KutnerUri · 2018-11-16T18:56:08Z

ok so I went ahead and did that. But I don't seem to be able to push?
not to a new branch or pr-125.

Should I clone the project and send a pull request to master or something like that?

adrianheine · 2018-11-16T19:41:56Z

Yes, you have to clone the project and open a PR to master.

KutnerUri · 2018-11-18T10:38:09Z

created - #170
I will add more tests when I have the time.

I am thinking about await outside a an async function:
This makes sense:

async function f(){
    return await Promise.resolve(3);
}

But this does not:

const a = await Promise.resolve(3);

It breaks code symmetry, and will require scaffolding when working with code fragments.

So, I'm suggesting that Bublé will support this, but with a flag. It could be topLevelAwait or standaloneAwait:

const { code } = Buble.transform('const a = await f();', {transforms:{toLevelAwait: true}});
console.log(code);
//var a; Promise.resolve().then(function() { return f(); }).then(function(){ a= f(); return a; }) ;

adrianheine · 2018-11-18T16:12:58Z

Current master should already support top-level await. I don't think there's need for an option.

leeoniya · 2018-11-18T19:28:27Z

not to derail the effort here, but there's a babel plugin with extensive support that might be easier to port than duplicating the effort to add all cases it already handles?

https://github.com/rpetrich/babel-plugin-transform-async-to-promises

aleclarson · 2020-04-24T14:08:56Z

When running yarn test on this PR, I get this error:

Transforming async functions is not implemented. Use `transforms: { asyncAwait: false }` to skip transformation and disable this error. (1:0)

@adrianheine Do you know how I could fix this?

edit: Nevermind, I think I can figure it out.

This was referenced Apr 10, 2018

Support for async functions #90

Open

Invalid output for async arrow function (expression) #109

Closed

adrianheine mentioned this pull request Apr 12, 2018

Dariocravero/buble async await #12

Closed

ulivz mentioned this pull request Apr 18, 2018

Async / await vuejs/vuepress#124

Closed

aleclarson reviewed May 14, 2018

View reviewed changes

rtibbles mentioned this pull request May 16, 2018

Upgrade Node 6.x => 8.x (LTS) learningequality/kolibri#3653

Closed

sisou mentioned this pull request Jul 23, 2018

Invalid syntax when transpiling async methods on classes #146

Closed

jimthedev mentioned this pull request Aug 29, 2018

Next: Plan future API FormidableLabs/react-live#89

Closed

StephanHoyer reviewed Oct 26, 2018

View reviewed changes

adrianheine reviewed Nov 1, 2018

View reviewed changes

curran reviewed Nov 13, 2018

View reviewed changes

dy mentioned this pull request Nov 15, 2018

Introduce tests & CI gl-vis/regl-scatter2d#15

Merged

adrianheine force-pushed the async branch from 578a849 to 40799cb Compare November 15, 2018 20:04

adrianheine requested changes Nov 15, 2018

View reviewed changes

adrianheine reviewed Nov 15, 2018

View reviewed changes

Support Async/Await based on bublejs#12

417ba2a

adrianheine force-pushed the async branch from 40799cb to 417ba2a Compare November 16, 2018 16:43

leeoniya mentioned this pull request Nov 18, 2018

port plugin to Buble? rpetrich/babel-plugin-transform-async-to-promises#16

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Async/Await based on #12 #125

Support Async/Await based on #12 #125

shirotech commented Apr 10, 2018 •

edited

Loading

shirotech commented Apr 12, 2018

aleclarson commented May 14, 2018 •

edited

Loading

aleclarson May 14, 2018

maxmilton Aug 31, 2018 •

edited

Loading

adrianheine Nov 1, 2018

mrTimofey commented Aug 7, 2018

aleclarson commented Aug 31, 2018

KutnerUri commented Oct 14, 2018 •

edited

Loading

StephanHoyer Oct 26, 2018

shirotech commented Oct 31, 2018

adrianheine Nov 1, 2018

adrianheine Nov 1, 2018

adrianheine Nov 1, 2018

curran left a comment

curran Nov 13, 2018

curran Nov 13, 2018

curran Nov 13, 2018

KutnerUri Nov 16, 2018

dy commented Nov 15, 2018

curran commented Nov 15, 2018

adrianheine commented Nov 15, 2018

adrianheine Nov 15, 2018

adrianheine Nov 15, 2018

adrianheine Nov 15, 2018

adrianheine commented Nov 16, 2018

KutnerUri commented Nov 16, 2018

adrianheine commented Nov 16, 2018

KutnerUri commented Nov 16, 2018

KutnerUri commented Nov 16, 2018 •

edited

Loading

adrianheine commented Nov 16, 2018

KutnerUri commented Nov 18, 2018 •

edited

Loading

adrianheine commented Nov 18, 2018

leeoniya commented Nov 18, 2018

aleclarson commented Apr 24, 2020 •

edited

Loading

Support Async/Await based on #12 #125

Are you sure you want to change the base?

Support Async/Await based on #12 #125

Conversation

shirotech commented Apr 10, 2018 • edited Loading

shirotech commented Apr 12, 2018

aleclarson commented May 14, 2018 • edited Loading

Choose a reason for hiding this comment

maxmilton Aug 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrTimofey commented Aug 7, 2018

aleclarson commented Aug 31, 2018

KutnerUri commented Oct 14, 2018 • edited Loading

Choose a reason for hiding this comment

shirotech commented Oct 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

curran left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dy commented Nov 15, 2018

curran commented Nov 15, 2018

adrianheine commented Nov 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrianheine commented Nov 16, 2018

KutnerUri commented Nov 16, 2018

adrianheine commented Nov 16, 2018

KutnerUri commented Nov 16, 2018

KutnerUri commented Nov 16, 2018 • edited Loading

adrianheine commented Nov 16, 2018

KutnerUri commented Nov 18, 2018 • edited Loading

adrianheine commented Nov 18, 2018

leeoniya commented Nov 18, 2018

aleclarson commented Apr 24, 2020 • edited Loading

shirotech commented Apr 10, 2018 •

edited

Loading

aleclarson commented May 14, 2018 •

edited

Loading

maxmilton Aug 31, 2018 •

edited

Loading

KutnerUri commented Oct 14, 2018 •

edited

Loading

KutnerUri commented Nov 16, 2018 •

edited

Loading

KutnerUri commented Nov 18, 2018 •

edited

Loading

aleclarson commented Apr 24, 2020 •

edited

Loading