always wrap function

[iwestlin]

Consider this:

const asy = require('../dist/async')

const arr = [1, 2, 3]
const sleep = ms => new Promise(res => setTimeout(res, ms))
const fn = v => {
  console.log({v})
  return sleep(v)
}
asy.eachLimit(arr, 2, fn).then(() => {
  console.log('done')
}).catch(e => console.error('err:', e))

will print

{ v: 1 }
{ v: 2 }

and eachLimit will never resolve or reject.

This can be fixed by manually add async keyword like

const fn = async v => {

because this module uses Symbol.toStringTag to determin if a function is an async function.

However fn can be an async function even if it doesn't have an async keyword, because it can return a Promise.

So my fix is to wrap all iteratee function no matter it has an async keyword or not.

By doing so, eachLimit will be resolved:

{ v: 1 }
{ v: 2 }
{ v: 3 }
done

But I'm not sure if this change will effect other functions, so be careful if you are gonna merge...😅

[iwestlin]

Ahh...I just ran npm run test, and it failed of course... It required a lot more work than this...😅

[iwestlin]

I managed to find a way to resolve promises while passing all the tests: https://github.com/iwestlin/async/commit/a8f6a17352274f4526cae1e52dfec07831055815

But I don't think it's a proper fix because it only works on eachLimit

[iwestlin]

Finally I found a way to support normal functions which return Promise, while passing all the tests, check out file changes: https://github.com/caolan/async/pull/1895/files

[aearly]

We've been down this road many times before, and unfortunately it's not possible to do this cleanly. The main problem is that if a function returns a then-able, you don't want to pass it a callback. However, you have to call the function first to see if it returns a Promise/then-able, and you can't retroactively not pass a callback to it.