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

.each() synchronization question #513

Closed
ericlu88 opened this issue Feb 27, 2015 · 4 comments
Closed

.each() synchronization question #513

ericlu88 opened this issue Feb 27, 2015 · 4 comments

Comments

@ericlu88
Copy link

@ericlu88 ericlu88 commented Feb 27, 2015

We encountered the following synchronization problem while using .each(). We couldn't figure out if it is intentionally design like this or a bug based on the documentation, so would like to get any insight from you guys:

// Expect: 1 2 3 4 5
// Actual: 1 3 4 5 2
console.log(1);
Promise.resolve([3,4,5])
    .each(function(v) {
        console.log(v);
    });
console.log(2);

It seems like the function get executed synchronously. The same problem happens to .filter() function too. In the contrary , the following snippet get executed expectedly:

// Expect: 1 2 [3 4 5]
// Actual: 1 2 [3 4 5]
console.log(1);
Promise.resolve([3,4,5])
    .then(function(v) {
        console.log(v);
    });
console.log(2);
@petkaantonov
Copy link
Owner

@petkaantonov petkaantonov commented Feb 27, 2015

It's definitely a bug, the reduce implementation in 2.x is notoriously complicated and each delegates to it as well hence it is affected. The implementation has been redone in 3.x and it shouldn't have this bug. If this is critical for you I can make a patch for 2.9.13.

@ericlu88
Copy link
Author

@ericlu88 ericlu88 commented Feb 27, 2015

Thanks for getting back super fast. Right now we are using a work around by chaining a .then() as a pass through to make it really async. It would be awesome if we can have a patch release soon. Also, when are you planning for the 3.x release?

@petkaantonov
Copy link
Owner

@petkaantonov petkaantonov commented Feb 27, 2015

Fix is in 2.9.13 release just published to npm. 3.x will be released probably 1-3 weeks from now.

@ericlu88
Copy link
Author

@ericlu88 ericlu88 commented Feb 27, 2015

Cool, just verified the issue get fixed. Thanks a lot!

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.