Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic support for await with the pipeline operator #7154

Closed
wants to merge 7 commits into from
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const awaitVisitor = {

AwaitExpression(path, { wrapAwait }) {
const argument = path.get("argument");
argument.node.__wasAwait = true;
Copy link
Member

Choose a reason for hiding this comment

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

I think we wanted to stay away from using a hidden properties? Or we need a more formalized way to handle this kind of thing (like knowing if it was transformed or original source, and what it used to be)

Copy link
Author

Choose a reason for hiding this comment

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

I agree. Do you have any suggestions that could help either act as an indicator or force these two plugins to run in order that wouldn't require blocking until those issues are solved at a higher level?

Copy link
Member

@xtuc xtuc Mar 8, 2018

Choose a reason for hiding this comment

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

Unfortunately no, it's related to #5854

Inside the same plugin you can use a WeakMap/WeakSet.

Copy link
Member

Choose a reason for hiding this comment

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

Plugin ordering definitely makes this harder. Could we make this throw an error if the async function tries to transform an await that is inside of a pipeline expression, and then have the pipeline transform have a visitor for async functions that does an explicit sub-traversal for pipeline expressions? Then we could just make sure that the pipeline plugin runs before the async function transform and I think we'd be okay?

Copy link
Member

Choose a reason for hiding this comment

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

That sounds good to me, we don't need to order the plugins if they are both idenpendent. Each will transform what it needs.


if (path.parentPath.isYieldExpression()) {
path.replaceWith(argument.node);
Expand Down
13 changes: 12 additions & 1 deletion packages/babel-plugin-proposal-pipeline-operator/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export default function() {

let optimizeArrow =
t.isArrowFunctionExpression(right) && t.isExpression(right.body);
const wasAwait = right.argument && right.argument.__wasAwait;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe

const wasAwait = t.isYieldExpression(right) && right.argument && right.argument.__wasAwait;

?

Copy link
Author

Choose a reason for hiding this comment

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

Probably a good idea.

const isAwait = t.isAwaitExpression(right) || wasAwait;
let param;

if (optimizeArrow) {
Expand All @@ -24,6 +26,11 @@ export default function() {
} else if (params.length > 0) {
optimizeArrow = false;
}
} else if (isAwait) {
right.argument = t.sequenceExpression([
t.numericLiteral(0),
right.argument,
]);
} else if (t.isIdentifier(right, { name: "eval" })) {
right = t.sequenceExpression([t.numericLiteral(0), right]);
}
Expand All @@ -44,7 +51,11 @@ export default function() {

const call = optimizeArrow
? right.body
: t.callExpression(right, [placeholder]);
: isAwait
? t[wasAwait ? "yieldExpression" : "awaitExpression"](
t.callExpression(right.argument, [placeholder]),
)
: t.callExpression(right, [placeholder]);
path.replaceWith(
t.sequenceExpression([
t.assignmentExpression("=", placeholder, left),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
var incPromise = (x) => Promise.resolve(x + 1);
Copy link
Member

Choose a reason for hiding this comment

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

i'd prefer to have the same code in actual / exec if they are in the same directory, exec differs slightly right now and it makes it hard to compare those files (assert ofc should stay just in exec)

Copy link
Author

Choose a reason for hiding this comment

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

I made them match now. I think I may have saved the file with Prettier on at some point.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I didnt mean that they do not match in sense of formatting - i think there was some differenc in how incFuncPromise was used

Copy link
Author

Choose a reason for hiding this comment

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

Oh, got it. I also changed the usage of that variable down below. Either way, they're the same now.

Copy link
Member

Choose a reason for hiding this comment

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

Cool!

var incFuncPromise = Promise.resolve(x => x + 10);
var double = (x) => x * 2;

var result = async () => 10 |> await incPromise;
var result2 = async () => 10 |> await incPromise |> double;

function* foo() {
return 42 |> (yield incFuncPromise);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
var incPromise = x => Promise.resolve(x + 1);
var incFuncPromise = Promise.resolve(x => x + 10);
Copy link
Member

Choose a reason for hiding this comment

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

This is not used.

Copy link
Author

Choose a reason for hiding this comment

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

Removed!

var double = x => x * 2;

var result = async () => 10 |> await incPromise;
var result2 = async () => 10 |> await incPromise |> double;

function* foo() {
return 42 |> (yield 10);
}

return Promise.all([result(), result2()]).then((results) => {
var r = results[0];
var r2 = results[1];
assert.equal(r, 11);
assert.equal(r2, 22);

var fooGen = foo();
var amount = fooGen.next().value;
assert.equal(fooGen.next(x => x + amount).value, 52);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
var incPromise = x => Promise.resolve(x + 1);

var incFuncPromise = Promise.resolve(x => x + 10);

var double = x => x * 2;

var result =
/*#__PURE__*/
function () {
var _ref = babelHelpers.asyncToGenerator(function* () {
var _;

return _ = 10, yield (0, incPromise)(_);
});

return function result() {
return _ref.apply(this, arguments);
};
}();

var result2 =
/*#__PURE__*/
function () {
var _ref2 = babelHelpers.asyncToGenerator(function* () {
var _ref3, _2;

return _ref3 = (_2 = 10, yield (0, incPromise)(_2)), double(_ref3);
});

return function result2() {
return _ref2.apply(this, arguments);
};
}();

function* foo() {
var _3;

return _3 = 42, (yield incFuncPromise)(_3);
Copy link
Member

Choose a reason for hiding this comment

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

was this intentional? looks weird, but i guess it could test something, but then exec should reflect that test

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"plugins": ["proposal-pipeline-operator", "external-helpers", "transform-async-to-generator"],
"parserOpts": {
"allowReturnOutsideFunction": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
var incPromise = (x) => Promise.resolve(x + 1);
var double = (x) => x * 2;

var result = async () => 10 |> await incPromise;
var result2 = async () => 10 |> await incPromise |> double;
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
var incPromise = x => Promise.resolve(x + 1);
var double = x => x * 2;

var result = async () => 10 |> await incPromise;
var result2 = async () => 10 |> await incPromise |> double;

return Promise.all([result(), result2()]).then(([r, r2]) => {
assert.equal(r, 11);
assert.equal(r2, 22);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
var incPromise = x => Promise.resolve(x + 1);

var double = x => x * 2;

var result = async () => {
var _;

return _ = 10, await (0, incPromise)(_);
};

var result2 = async () => {
var _ref, _2;

return _ref = (_2 = 10, await (0, incPromise)(_2)), double(_ref);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"minNodeVersion": "7.6.0",
"parserOpts": {
"allowReturnOutsideFunction": true
}
}