Skip to content
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

reduce callback isn't really optional #1223

Closed
asquared opened this issue Jul 9, 2016 · 6 comments
Closed

reduce callback isn't really optional #1223

asquared opened this issue Jul 9, 2016 · 6 comments
Milestone

Comments

@asquared
Copy link

asquared commented Jul 9, 2016

What version of async are you using?
1.5.2 (but the offending code seems to still exist in master)

Which environment did the issue occur in (Node version/browser version)
node v0.10.29

What did you do? Please include a minimal reproducable case illustrating issue.
The documentation says the callback is optional, so I should be able to do:

const async = require('async');

var array = [1, 2, 3];
function reducer(memo, item, callback) {
        memo += item;
        console.log("sum is %d", memo);
        setImmediate(callback, null, memo);
}

async.reduce(array, 0, reducer);

What did you expect to happen?

sum is 1
sum is 3
sum is 6

What was the actual result?

sum is 1
sum is 3
sum is 6

.../node_modules/async/lib/async.js:380
            callback(err, memo);
            ^
TypeError: undefined is not a function
    at .../node_modules/async/lib/async.js:380:13
    at .../node_modules/async/lib/async.js:52:16
    at .../node_modules/async/lib/async.js:269:32
    at .../node_modules/async/lib/async.js:44:16
    at Object.<anonymous> (.../node_modules/async/lib/async.js:377:17)
    at Object.immediate._onImmediate (timers.js:354:16)
    at processImmediate [as _immediateCallback] (timers.js:336:15)
@megawac
Copy link
Collaborator

megawac commented Jul 9, 2016

Yeah, actually looking into this a lot of methods are documented to have optional callbacks when they are clearly required

These are the ones I've identified are incorrectly documented:

  • filter*
  • map*
  • reduce*
  • sortBy
  • transform

@megawac
Copy link
Collaborator

megawac commented Jul 9, 2016

Should these be optional or not @aearly

@asquared
Copy link
Author

asquared commented Jul 9, 2016 via email

@megawac megawac added this to the 2.0 milestone Jul 10, 2016
@aearly
Copy link
Collaborator

aearly commented Jul 10, 2016

Yeah, they should all be optional. I do think it's odd that you would use map or filter and then ignore the result, but we should make the callback optional for consistency with other methods.

@megawac
Copy link
Collaborator

megawac commented Jul 10, 2016

I think sortBy should be the only one required -- any objection @aearly?

@aearly
Copy link
Collaborator

aearly commented Jul 10, 2016

Yeah, there'd be no reason to use sortBy and ignore the result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants