-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix callback argument order in async.auto #1036
Comments
Bad idea. For new ordering should be new function and "auto" stay like a flip wrapper for this (mark as deprecated). |
One of the themes of the 2.0 release is standardizing the types of functions we use. This will be a well documented breaking change. I think breaking with node's async conventions originally was a worse idea. |
A config flag won't work well -- say all your functions expect the args flipped, but another node module expects the args un-flipped. If Async is de-duped to the same module, that config would conflict. Wrapping all your dependent auto tasks with We didn't just arbitrarily make this change for the sake of standardization -- we made it so other higher-order functions would work properly. (e.g. directly passing the result of |
This was a bad idea imho :/ Now you can't just define a |
I think the consistency in 2.x is nice, but I agree that there are advantages to having the callback first. It's probably not worth changing at this point, but always having the callback first across the entire library, like the error argument, may have been a better design. |
We should standardize the signature of async functions used in
async.auto
. Rather than always having the callback first, and the optionalresults
object second, we should always make the callback the last argument.We could also include a
flip
function that swaps the 2 arguments, to ease migration. (Or just instruct people to_.rearg(asyncFn, 1, 0)
).The text was updated successfully, but these errors were encountered: