Irakli says that Q has it now, so my 'lets not jump ahead of Q' objection is gone. I vote we add it.

See https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/lib/promise.js#L179

Honza - this answers the promise grouping question that you were asking about.

If we're opening this up, I would propose 2 further changes: all: Turns an array of promises into a promise for an array. If any of the promises gets rejected, the whole array is rejected immediately. Justification: Using 'promised' for all is ugly, so everyone is going to be using a shortcut anyway. Better to provide one ourselves. then should have the signature then(callback, errback, progress) I think that all 3 changes can be made in a way that is 100% compatible with Q, jetpack and mozilla-central.

> all: Turns an array of promises into a promise for an array. If any of the promises > gets rejected, the whole array is rejected immediately. From our experience one rarely needs `all` at all, since it's just possible to write functions that take promises as arguments. On the other hand bigger API surface makes learning curve steeper. So I'd be against including this into core, that's not to imply my opinions on other teams, I'd suggest implementing another module with a larger utility belt for you needs, same as we did with `promised` decorator for jetpack. > then should have the signature then(callback, errback, progress) You could pass `progress` listener to `then` if you want to, although it will be ignored. I'm also convinced that promises are wrong data structures for representing streaming data, I believe we discussed that and Yorik agreed on this too. Also as far as I know `progress` is not in `Q`. I'd suggest use of linked list of promises for representing streaming data, or even better something like Microsoft's RX http://msdn.microsoft.com/en-us/data/gg577609.aspx or clojure like reducers https://github.com/Gozala/reducers or event emitters. If you're still convinced that promises with progress listener is a best way to go about it, I would again prefer if you built it on top of promise/core without implying use of it on others.

Either way I'd prefer to collaborate on each individual addition separately.

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #5) > Either way I'd prefer to collaborate on each individual addition separately. I second this. Let's open one bug per feature. (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #4) > > all: Turns an array of promises into a promise for an array. If any of the promises > gets rejected, the whole array is rejected immediately. > > From our experience one rarely needs `all` at all, since it's just possible > to write functions that take promises as arguments. On the other hand bigger > API surface makes learning curve steeper. So I'd be against including this > into core, that's not to imply my opinions on other teams, I'd suggest > implementing another module with a larger utility belt for you needs, same > as we did with `promised` decorator for jetpack. I personally dislike `promised` and consider `all` much more usable by newcomers. However, given that they are essentially equivalent, if as I understand `promised` is part of the standard, I would go for `promised` and not `all`. Given that `promised` is complex, though, we need kick-ass documentation! > > then should have the signature then(callback, errback, progress) [...] Joe, could you open a bug on that topic?

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #4) > > all: Turns an array of promises into a promise for an array. If any of the > promises gets rejected, the whole array is rejected immediately. > > From our experience one rarely needs `all` at all, since it's just possible > to write functions that take promises as arguments. You're creating APIs, I'm consuming them, which might be the difference. We will have a utility for this if you don't. > On the other hand bigger > API surface makes learning curve steeper. I don't agree, because it's one of the first questions I've been asked, and the alternative isn't at all obvious. (In reply to David Rajchenbach Teller [:Yoric] from comment #6) > I personally dislike `promised` and consider `all` much more usable by > newcomers. However, given that they are essentially equivalent, if as I > understand `promised` is part of the standard, I would go for `promised` > and not `all`. IIRC, both are in Q. (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #4) > > then should have the signature then(callback, errback, progress) > > You could pass `progress` listener to `then` if you want to, although it > will be ignored. I'm also convinced that promises are wrong data structures > for representing streaming data, I believe we discussed that and Yorik > agreed on this too. Also as far as I know `progress` is not in `Q`. progress is in Q, and passing a progress listener to then doesn't work. > I'd suggest use of linked list of promises for representing streaming data, This isn't about steaming data. This is about progress notification.

I'd like to re-animate this bug. To be clear on the request that we're making here: Let's include "promised()" and "all()" in our implementation of promise. The question isn't "should we have them?" because they're in central at least once already. I think we want to avoid different implementations of these handy methods popping up everywhere.

Every promise library I have used has had some kind of helper for promises of arrays. We need this, or else we will end up with many disparate implementations. I had to add my own for a recent patch: /** * Takes an array of promises and reduces the values that they resolve to. Like * Array.prototype.reduce, but for arrays of promises, and returns a promise of * the final accumulated value. */ function resolveReduce(aPromises, aAccumulation, aReducer) { if (aPromises.length === 0) { return resolve(aAccumulation); } else { return resolve(aPromises[0]).then(function (val) { return resolve(aReducer(val, aAccumulation)).then(function (newAccumulation) { return resolveReduce(aPromises.slice(1), newAccumulation, aReducer); }, reject); }, reject); } } /** * Takes an array of promises, resolves each of them, and returns a new promise * that resolves to an array of each of those resolved values from the original * array of promises. */ function resolveAll(aPromises) { return resolveReduce(aPromises, [], function (val, acc) { acc.push(val); return acc; }); }

Note that Task.jsm basically trivializes waiting for any set of promises. let promises = // Some array let results = []; let promised = Task.spawn(function() { for (let p of promises) { results.push(yield p); } });

Since `promised` is already exposed I'm renaming this to just exposing `Q.all` compatible `all` function from promise module. For the reference of Q.all API see: http://documentup.com/kriskowal/q/#tutorial/combination

Pointer to Github pull-request

Pointer to Github pull-request

Looks good to me. Thanks.

Comment on attachment 722462 [details] GH Pull Request 842 Looks good, thanks!

Commits pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/87737e7f080a8464e40a06c12534e648b1abca82 Bug 790195: Expose Promise.all sugar for promised(Array) https://github.com/mozilla/addon-sdk/commit/ef7e1c3a7d2f758496b473b77f251afc1d7e599c Merge pull request #842 from jsantell/Q-all-790195 Fix Bug 790195: Expose Promise.all, sugar for promised(Array), r=@gozala

Commit pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/e468f3e947df475300719da6dd199fbe3d024551 Bug 790195: Expose Promise.all sugar for promised(Array)