cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Regression between 3.6.0 and 3.6.1 probably related to `.filter` and `Promise<boolean>` #400

Closed mikaelkaron closed 9 years ago

mikaelkaron commented 9 years ago

Let's try short and in bullet form:

EyalAr commented 9 years ago

The problem can be reduced to the following inconsistency between 3.6.0 and 3.6.1:

var when = require('when'),
    nums = [1, 2, 3, 4];

when.filter(nums, function(n, i){

    var d = when.defer(),
        p = d.promise;

    setTimeout(function(){
        d.resolve(n % 2 === 0);
    }, 0);

    return p.tap(function(){
        delete nums[i];
    });

}).then(function(result){
    console.log(result);
});

Running this on 3.6.0 will print [ 2, 4 ], while running on 3.6.1 will print [ undefined, undefined ].

briancavalier commented 9 years ago

Thanks for the bisect and the runnable test case! I'll look into this today and see what's going on.

briancavalier commented 9 years ago

@mikaelkaron @EyalAr Can you try this branch to see if it helps your use cases?

It passes @EyalAr's test case for me. I have to admit that I didn't consider the case where the filtering predicate would forcibly modify the original array--we didn't have a unit test for that. While my personal feeling is it's a strange thing to do, Array.prototype.filter does allow it, ie if you translate the example above into synchronous code using Array.prototype.filter, it will indeed log [2, 4]. So, it makes sense that when.filter should match that behavior.

Let me know if that branch helps, and if so, I'll get a 3.6.2 patch release out today. Thanks!

briancavalier commented 9 years ago

Unit test added in 8a15579. Test fails in 3.6.1, but passes in that commit.

mikaelkaron commented 9 years ago

This does indeed fix our issue, thanks!

briancavalier commented 9 years ago

@mikaelkaron Great, I'll get that merged in asap. We're targeting a 3.6.2 release within the next day.