async Function Support

[aearly]

One of the elephants in the room is the new async/await support that has come to Node and Chrome, and will soon hit every other major browser. I've been thinking about what Async can do in the async/await world.

Currently, we can adapt async functions by wrapping them with asyncify. Since an async function is essentially just a function that returns a Promise, that old adapter can easily convert it to a callback-style function. However, it leads to the somewhat absurd looking:

async.mapLimit(arr, 10, async.asyncify(async (val) => {
  let foo = await doSomething(val);
  //...
 return bar;
}), done);

However, one of the features in the spec for async functions is that:

Object.getPrototypeOf(asyncFn)[Symbol.toStringTag] === "AsyncFunction"

This gives a way to easily detect (native) async functions. We could use this technique to automatically asyncify them. The example above becomes:

async.mapLimit(arr, 10, async (val) => {
  let foo = await doSomething(val);
  //...
 return bar;
}, done);

...which seems to flow much more naturally. I also think we should continue to use callbacks. If a user wanted to await the result, they would have to promisify the function, or pify Async as a whole:

let result = await pify(async.mapLimit)(arr, 10, async (val) => {
  let foo = await doSomething(val);
  //...
 return bar;
});

The above method for detecting async functions only works with native functions. I don't think there is a way to detect Babel transpiled functions. We certainly can't detect normal functions that simply return Promises, because we'd have to retroactively not pass a callback. There would he a huge caveat that this would only work without a transpiler in very modern environments, otherwise you still have to manually wrap with asyncify.

Also, admittedly, many Async methods don't make sense with async/await. Most of the control flow methods (save for things like auto and queue) are more easily replicated with native control flow constructs. map and parallel can be replaced with Promise.map and Promise.all. However, the limiting collection functions would be very useful, as well as auto and a few others. (Also, autoInject with async functions is a async control flow dream!)

[megawac]

I'm going to ponder over this some more but I'd like to ask a couple technical questions just so I can try to better envision what this looks like..

Is there a reason to do Object.getPrototypeOf(asyncFn)[Symbol.toStringTag] === "AsyncFunction" or can we do asyncFn[Symbol.toStringTag] === "AsyncFunction" (seems to work in FF)?

So is the proposal any time someone provides a callback of the format cb(err, arg) we should detect if it is an AsyncFunction; if it is an async function we should apply promisify otherwise use it as is

Also sorry, I'm not following on the await example, if we detect that the function is an AsyncFunction what are the challenges of supporting await?

[aearly]

Is there a reason to do Object.getPrototypeOf(asyncFn)[Symbol.toStringTag] === "AsyncFunction" or can we do asyncFn[Symbol.toStringTag] === "AsyncFunction" (seems to work in FF)?

Thats just the canonical ECMA spec way to do it. I guess in theory, someone could overwrite asyncFn[Symbol.toStringTag].

So is the proposal any time someone provides a callback of the format cb(err, arg) we should detect if it is an AsyncFunction; if it is an async function we should apply promisify otherwise use it as is

I think you have it a bit backwards. Wherever we accept an callback-accepting iteratee function (function(args..., callback) {}), we should check to see if it is an async function, and then asyncify it.

The await example is what someone would have do if they wanted to await an Async method. I don't think we should have Async methods start returning promises so that would work -- leave it for the user to do.

[aearly]

Implemented in #1390 !

[manvalls]

This was a breaking change and broke our deployed code. Please think twice when doing such things without increasing the major version.

PS: thanks for all the great work you're doing with this library 😄

[megawac]

Shoot! What broke. Could you please create a ticket with details of the environment this didn't work in? Thanks!

…

On Wed, Apr 5, 2017 at 10:18 AM Manuel Valls Fernández < ***@***.***> wrote: This was a breaking change and broke our deployed code. Please think twice when doing such things without increasing the major version. PS: thanks for all the great work you're doing with this library 😄 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1386 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADUIEPNkTSOVuuiwucBVrH983X6B568Wks5rs6K3gaJpZM4Mf64R> .

[sergiofgonzalez]

Agreed, it broke my build too...

A waterfall that was calling an async function which worked a few days back started failing with a "cb is not function" because the async callback was no longer provided to the function.

[aearly]

Sorry we broke your code. We didn't anticipate your use case for async functions. I'd recommend rolling back to 2.2.0, or refactoring your code to return the values you need, rather than using callbacks. Unfortunately, the cat is out of the bag with this feature, so we can't roll back.

[sergiofgonzalez]

@aearly Please, don't mention it!! It's really nice of you to answer 🥇

@manvalls hinted me a great solution which did not require to rollback. As you're using the symbol to detect the async in the function declaration, he thought of a clever way to cheat the detection.

My waterfall was using functions exported from other modules, one of them was an async one and thus causing the failure.

So just by changing from:

... 
/* services module */
function doThis(param, cb) {
...
}

async function doThatAsync(param, cb) {
...
}

module.exports = {
  doThis: doThis,
  doThat: doThatAsync  
}; 

...
async.waterfall([
  services.doThis,
  services.doThat,  // fails with "cb is not a function"
], err => {
...
}

To:

... 
/* services module */
function doThis(param, cb) {
...
}

async function doThatAsync(param, cb) {
...
}

module.exports = {
  doThis: doThis,
  doThat: (...args) => doThatAsync(..args)   // cheating the detection
}; 

...
async.waterfall([
  services.doThis,
  services.doThat, /* it works!!! */
], err => {
...
}

Thank you very much again

[ORESoftware]

can we use async/await with async.autoInject()?

async.autoInject({
    
    conn1: async function () {
      return conn1;
    },
    
    conn2: async function () {
      return conn2;
    },
});

doesn't seem to work, I get:

Error: autoInject task functions require explicit parameters.
at /Users/alexamil/WebstormProjects/nabisco/cdt-now/node_modules/async/dist/async.js:2081:23
at /Users/alexamil/WebstormProjects/nabisco/cdt-now/node_modules/async

[hargasinski]

@ORESoftware yes, async functions should work with autoInject. I tested the code you posted in Chrome, and it ran. I got a ReferenceError in the final callback as conn1 and conn2 are undefined. After changing it to

async.autoInject({
    conn1: async function () {
      return 'foo'
    },
    conn2: async function () {
      return 'bar'
    },
})

it works fine. However, we don't support transpiled async functions. Are you transpiling your code?