babel / kneden

Transpile ES2017 async/await to vanilla ES6 Promise chains: a Babel plugin
ISC License
514 stars 41 forks source link

Incorrect result with async arrow functions when not using `babel-preset-es2015` #19

Open latentflip opened 8 years ago

latentflip commented 8 years ago

I tried kneden out on some test files where I was previously using babel-transform-async-to-generator and came across a failing case.

In this project I wasn't using babel-preset-es2015 as I just wanted to compile async/await down to es6.

I've simplified the test case down to this:

.babelrc

{
  "plugins": ["../../../src"]
}

actual.js

function test() {
  let server;
  before(async () => server = await TestServer.start());
}

The output I get from running the test is:

function test() {
  let server;
  before(() => {
    return Promise.resolve();
  });
}

which is definitely not right :)

If I add "presets": ["es2015"] to .babelrc in the fixture dir I get the more correct looking:

function test() {
  var server = undefined;
  before(function () {
    return Promise.resolve().then(function () {
      return TestServer.start();
    }).then(function (_resp) {
      return server = _resp;
    });
  });
}
marten-de-vries commented 8 years ago

Thanks for your bug report. Yes, es2015 is necessary when using ES6 functionality like arrow functions. It doesn't make much sense to duplicate all of Babel's functionality in this plugin. It compiles ES5 with async/await to ES5 with promises, and that's it. (Although it does take care to run after the es2015 preset so it can be used in conjunction with it)

It might be good to clarify that somewhere in the README file. Keeping the issue open for that, PRs welcome if you want too speed that up a bit. :)

marten-de-vries commented 8 years ago

Wait, you're running this in an ES6 environment... Yes, I would like to support that use case, but not sure how to do it. Converting arrow functions too is probably pretty doable, but e.g. doing variable hoisting which is required with let/const is more difficult than using var. So you'd still need to run the block scope plugin of babel anyway probably. Need to look into it a bit more.

Why are you using kneden in this case? Because it's probably pretty hard to beat the readability of babel-transform-async-to-generator in an env that supports generators.

latentflip commented 8 years ago

Yeah, you're right, I was trying to run it in an es6 environment (node 4) and use Babel + this to just compile the async/await stuff rather than everything down to es5, but I can see that gets tricky with let/const

Philip Roberts

On 14 Feb 2016, at 11:28, Marten de Vries notifications@github.com wrote:

Wait, you're probably running this in an ES6 environment... Yes, I would like to support that use case, but not sure how to do it. Converting arrow functions too is probably pretty doable, but e.g. doing variable hoisting which is required with let/const is more difficult than using var. So you'd still need to run the block scope plugin of babel anyway probably. Need to look into it a bit more.

— Reply to this email directly or view it on GitHub.

marten-de-vries commented 8 years ago

Ok, thought a bit more about this. It would definitely be a long-term project, and considering babel-transform-async-to-generator's existence might not be worth it, but these would be the steps required:

latentflip commented 8 years ago

No worries, yeah to be honest it sounds like just mentioning in the docs that you need es2015 set for this to work is probably the best solution.

Why are you using kneden in this case? Because it's probably pretty hard to beat the readability of babel-transform-async-to-generator in an env that supports generators.

Mostly cos it looked like a neat project and I figured I'd throw it at something and see how it worked :)

marten-de-vries commented 8 years ago

Added a note to the docs, leaving this open for the outlined long term plan.