Pass key to iterator if async.each() iterator takes 3 arguments

[olalonde]

I have implemented a way to optionally pass a key/index argument to the async.each iterator.

async.each([1,2,3], function (val, index, callback) {});
async.each({a: 1, b: 2, c: 3}, function (val, key, callback) {});

It is backwards compatible since it omits the key/index argument for iterator functions that accept less than 3 arguments.

All tests pass.

See issue #72.

[ljharb]

instanceof Array is not reliable. you should use Array.isArray(), or Object.prototype.toString.call(arr) === '[object Array]'

[gobwas]

What about this feature? =)

[ljharb]

Still waiting for updates from @olalonde :-)

[caolan]

Async does not use function arity to alter behaviour, since it can be unreliable in some cases.

[ljharb]

@caolan when can function arity be unreliable? jQuery uses this technique, I believe.

[aearly]

fs.readFile(filename, [encoding], callback) (encoding is optional)

If you did something like async.each({file1: "file1", file2: "file2"}, fs.readFile, callback) it would break with your patch because it would pass "file1" as the encoding.

There is a PR open to add a forEachOf method (#168) that adds a separate method with the behaviour you want, it just needs docs and tests.

[caolan]

What @aearly said :)

[gobwas]

@aearly 's example will also be broken with the native forEach method:

["fileA.txt", "fileB.txt"].forEach(fs.readFileSync);

Cause the native will pass 0, 1 as the encodings.

I mean, the example with the fs interface is not giving us any reason to do not pass the index/key in the handler.

[aearly]

@gobwas You're right about how the native forEach works, but your reasoning is wrong. The native forEach fails with fs.readFileSync for the exact same reason async.each would fail with fs.readFile if you passed the index.

The way async.each works now works with fs.readFile with no modifications. The proposed change breaks that.

[gobwas]

@aearly yes, exactly, it will fall in the same reason. And thats why we can
ask ourselfs a question - does native forEach also bad designed with
passing index as second argument? :)

[gobwas]

@aearly Was reading your comment from mobile, and not catched the last sentence. I agree that this feature breaks backward compatibility. But, async now at version 0.9.0, which means, by semver, that it is still not stable. This is a technical question, how to minimize backward compatibility issues. I think the interface of native iterators could be the leading idea in this question.

[aearly]

#168 addresses your concerns -- it would be backwards compatible since it introduces a new method.

While async is technically 0.x which is "the wild west" as far as semver is concerned, async is still used by thousands of projects and a change like what you propose would break tons of people's apps. (Some people have argued that async is ready for 1.0, though: see #562.)

[gobwas]

@aearly Mostly I agree with you. But. When you will release 2.0 version of async, will it be compatible to the current version? And will tons of people's apps will be broken of it?