search

cujojs / when

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

Should "spread" resolve promises in array? #368

Closed brandoncarl closed 9 years ago

brandoncarl commented 10 years ago

I realize that jQuery is the bane of promise existence, but it is highlighting a problem I've found in using "spread". I wanted to bring it to your intention to determine whether it's intentional or not.

Effectively, when wrapping $.ajax, and spreading the response (data, res, xhr), I'm finding that when.js 's spread method automatically calls the then method of xhr, making it impossible to access the original xhr object (for this like status code).

I've created a jsFiddle highlighting the issue here: http://jsfiddle.net/x3fgamf1/3/. You'll have to look at the console to see the difference in the objects.

Edited to correct fiddle.

Thanks!

rkaw92 commented 10 years ago

Could you please provide a code example? The jsfiddle fragment doesn't seem to deal with the issue at hand ("// Helper to see results in fiddle. Not related to bug.") and it might prove vain to discuss without a concrete case.

briancavalier commented 10 years ago

Hey @brandoncarl. Ugh, yes, jQuery's xhr thenables are kind of a mess--sometimes they fulfill with themselves o_0. As @rkaw92 asked, could you make sure the fiddle shows the issue? Thanks!

brandoncarl commented 10 years ago

Oops, forgot to save...will update it in just a bit.

brandoncarl commented 10 years ago

Ok, the correct link is: http://jsfiddle.net/x3fgamf1/3/

briancavalier commented 10 years ago

Thanks @brandoncarl. Yes, when.js's spread does an extra level of resolution, to ensure the array itself contains only non-promise values. I think in this case I might use then and an object, perhaps like http://jsfiddle.net/bgovtnjo/1/

Another option would be to forego spread and do the apply yourself.

Or you could use rest.js instead of jQuery for ajax :) (shameless plug, sorry!) However, I understand that if jQuery is already being loaded, it may make sense to stick with it. Rest.js has some really nice features, though, and returns when.js promises by default. Might be worth a look.

FWIW, the nested resolution behavior is something of a throwback to 2.x for compatibility. I'm considering removing it for 4.0.

brandoncarl commented 9 years ago

Thanks @briancavalier. And no problem on the rest.js plug! I've been very impressed by the CujoJS libraries in general. At this point, our code base is a bit too large to consider such a fundamental change, but I'll keep in mind going forward!

briancavalier commented 9 years ago

I've been very impressed by the CujoJS libraries in general

Cool, thanks! Glad you're finding them to be useful. We have more on the way :)

our code base is a bit too large to consider such a fundamental change

I completely understand, no worries.