Queue push API not great to queue arrays

[moeriki]

Right now if you want to push an Array to a queue to have it handled as a whole (not item by item) you need to double wrap it.

const q = queue((anArrayArg, callback) => {
  // do stuff with array
  callback(null);
});

q.push([[1, 2, 3]]);

1. I find it really weird to have to double-wrap arrays.
2. If we're using the push / unshift terminology, it should match the behaviour of Arrays.

I think it should be changed to.

QueueObject.push(item_1 [, item_2, ...item_n ], callback);

I realize this is breaking and I don't expect it to be changed anytime soon. But I would like to propose this for a 3.0 change.

Thoughts?

[aearly]

I'd be inclined to keep it as-is. Arrays are special cases, and I think wrapping them is better than making push variadic. It's most painful when it's an array literal, but in most cases q.push([arr], cb) isn't too bad in my view.

[moeriki]

I think it's bad for readability both with array variable, or array literal. Actually I consider it worse with an array variable. Now I put a comment with my q.push([arrayValue]) to explain why I wrap it.

I don't see the downside of a variadic push and do see (aforementioned) downsides of as it is. I understand the upside of keeping it as-is is to not have a breaking change for rather a minor issue.

If no one else chimes in we're probably better of keeping it as-is.

[moeriki]

Also whenever 3.0 rolls around ES2015 array destructuring will be widely available. q.push(...arrayValues, cb) wouldn't be so bad.

Another alternative is to add q.pushAll() to push multiple values in <ES2015.

[hargasinski]

From personal experience with async.queue, the main use case is getting an array of ids from API, and then pushing those to the queue for processing. So, I'd be inclined to keep it the way it is too.

[megawac]

I don't mind q.push(...arrayValues, cb) personally. I think I'd be 👍 for that syntax. Although it gets really tricky for people trying to use ES5 syntax

[aearly]

After thinking about this more, I really think we should keep it as is. The callback to q.push is optional, meaning if we made it variadic, it's going to be ambiguous if the last argument is a value to push, or the callback. Granted, you could do some inferring with typeof val === 'function', etc., but there is a valid use case of pushing functions to a queue.

Even if we made the callback required, it would also makes the TypeScript/Flow guys a bit unhappy because then they can't properly type the function (can't have params after ...rest params). (see #1228)

[moeriki]

Agreed. Closed.