MatAtBread / fast-async

605 stars 21 forks source link

Losing const #40

Closed reconbot closed 6 years ago

reconbot commented 6 years ago

I've noticed we lost const in async functions when we use fast-async.

export async function indexPost ({ id }, opts) {
  const input = { ids: [id] }
  await indexPosts(input, opts)
  return { post: { id } }
}
// bluebird
let indexPost = exports.indexPost = (() => {
  var _ref41 = (0, _bluebird.coroutine)(function* ({ id }, opts) {
    const input = { ids: [id] };
    yield indexPosts(input, opts);
    return { post: { id } };
  });

  return function indexPost(_x22, _x23) {
    return _ref41.apply(this, arguments);
  };
})();
// fast-async
function indexPost({ id }, opts) {
  return new Promise(function ($return, $error) {
    var input;
    input = { ids: [id] };
    return indexPosts(input, opts).then(function ($await_109) {
      return $return({ post: { id } });
    }.$asyncbind(this, $error), $error);
  }.$asyncbind(this));
}

My babel config looks like this

{
  "babel": {
    "presets": [
      [
        "env",
        {
          "targets": {
            "node": "6.10"
          },
          "exclude": [
            "transform-regenerator",
            "transform-async-to-generator"
          ]
        }
      ]
    ],
    "plugins": [
      "syntax-async-functions",
      [
        "fast-async",
        {
          "useRuntimeModule": true
        }
      ],
      "add-module-exports",
      [
        "transform-object-rest-spread",
        {
          "useBuiltIns": true
        }
      ]
    ]
  }
}
reconbot commented 6 years ago

A reduced test case:

input

async function indexPostAsync ({ id }, opts) {
  const input = { ids: [id] }
  await indexPosts(input, opts)
  return { post: { id } }
}

function indexPost ({ id }, opts) {
  const input = { ids: [id] }
  return indexPosts(input, opts).then(() => {
    return { post: { id } }
  })
}

function indexPosts () {
  return Promise.resolve(true)
}

output

"use strict";

var _nodentRuntime = require("nodent-runtime");

var _nodentRuntime2 = _interopRequireDefault(_nodentRuntime);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

function indexPostAsync({ id }, opts) {
  return new Promise(function ($return, $error) {
    var input;
    input = { ids: [id] };
    return indexPosts(input, opts).then(function ($await_1) {
      return $return({ post: { id } });
    }.$asyncbind(this, $error), $error);
  }.$asyncbind(this));
}

function indexPost({ id }, opts) {
  const input = { ids: [id] };
  return indexPosts(input, opts).then(() => ({ post: { id } }));
}

function indexPosts() {
  return Promise.resolve(true);
}
matAtWork commented 6 years ago

This is an unfortunate but necessary consequence of the compiler. Consider the case:

const x = await y() ;

This translates (basically) to:

var x ;
y().then( _result => x = _result) ;

x cannot be a const here as it is assigned in the Promise callback, not when it is declared. Moving the declaration into the callback breaks the scoping rules (x could also be used outside the callback) and leaving it as const fails in the callback.

Nodent (and fast-async) do however issue a warning at compile time if a const is possibly assigned to other than in its declaration to allow you to examine the code by hand, but the run-time check for const is not possible.

matAtWork commented 6 years ago

...which incidentally should not change the operation or logic of your code (consts are still treated as consts), but the output will contain a var that is only every assigned to once before it is every referenced.

reconbot commented 6 years ago

That makes sense for variables being assigned from inside the promise chain, I didn't think of that. But the line

const input = { ids: [id] }

Shouldn't change should it?

matAtWork commented 6 years ago

To be honest, it doesn't have to, but the analysis of the code path of where a const is declared and used is non-trivial in async code, so it's much easier to map them all (maintaining the functionality and warning of a potential run time error) than to attempt to work out how the const is used.

It's also complicated by the fact that early ES5 implementations of const (notably Safari 9 and node < 6) didn't implement the const scope as per the ES6 spec, but simply as an ES5 var that failed assignment at runtime.

node -e "{ const x = 1 } console.log(x)" works in node <= 5.9.1 (ES5 engine), but (correctly in ES6) fails in node >= 6.0

In order to generate code that works with both ES5 and ES6 engines, const has to be eliminated as it's semantics depend on the run-time environment.

reconbot commented 6 years ago

Makes a lot of sense too. That had to be difficult to learn. =p

Thanks again!

matAtWork commented 6 years ago

NP. I learnt far too much about JS semantics doing this, and most of it wasn't good!