Add promise support

[balupton]

Would be great if this supported returned promise instances. Not sure if this is a task for this, or for taskgroup's task.

@crito as the promise expert, any ideas?

[crito]

Using Q I can easily turn such an API to a promised one using Q.nfbind:

Q = require 'q'
ambi = require 'ambi'
syncMeth = (x, y) -> x * y
asyncMeth = (x, y, next) ->
  setTimeout (-> next(null, x*y)), 0

syncMethP = Q.nfbind ambi, syncMeth, 5, 2
asyncMethP = Q.nfbind ambi, asyncMeth, 5, 2

syncMethP().then(console.log)

Wouldn't that be sufficient?

[balupton]

So the goal for this would be to have the following supported:

// Import
var ambi = require('ambi');
var result;

// Sample methods
var syncMethod = function(x,y){
    return x*y;
};
var asyncMethod = function(x,y,next){
    return setTimeout(function(){
        next(null,x*y);
    },0);
};
var promiseMethod = function(x,y){
    return promiseInstance; // ember.js prefers to return promises than to call callbacks
};

// Call the synchronous function asynchronously
result = ambi(syncMethod, 5, 2, function(err,result){ // ambi adds support for this asynchronous callback automatically
    console.log(err, result); // null, 10
});
console.log(result); // 10 - just like normal

// Call the asynchronous function asynchronously
result = ambi(asyncMethod, 5, 2, function(err,result){ // ambi doesn't do anything special here
    console.log(err, result); // null, 10
});
console.log(result); // setTimeout - just like normal

// Call the promise function asynchronously
result = ambi(promiseMethod, 5, 2, function(err,result){ // ambi adds support for returned promises automatically
    console.log(err, result); // null, 10
});
console.log(result); // setTimeout - just like normal

The problem is, I know there is usually .catch and .then, so we would need to add support for both. Is there a handler for catching both a completion event and an error event? And what do promises do about multiple emissions? If say a completion callback fires twice, does that mean then will fire twice?

[crito]

.then has the following signature: .then(onSuccess, onError). .catch is basically just a shortcut for .then(undefined, onError) and allows for nicer syntax. Both callbacks, onSuccess and onError take one argument, value for onSuccess and err for onError. So if you get a promise and a callback you can do something along that line:

ambi = (fn, next) ->
  fn().then(((val) -> next(null, val)), ((err) -> next(err)))

In Q at least you have a special method called done, that is like then, but errors don't bubble up the promise chain. Use it if you really want to finish a promise chain.

Promises get rejected or resolved only once. So your callback should only fire once unless you attach it twice.

Generally If I would use an API that supports promises as argument, I would expect that API to return me a promise as well. I would find it quite unnatural to supply a function with a promise and a callback. So something along this line is nicer:

ambi(promiseMethod).then(console.log, console.log)

Does this make sense even?

[balupton]

That makes sense.

The idea is for things like chainy and taskgroup, that I can start using ember things or promises as actions. Apparently the reason ember adopted promises is that people forget to call the callback, whereas because they are mostly coffeescript users, working with promises worked well as they could just code their promise stuff, the promise would be returned, and all would be well.

Having ambi return a promise seems like overkill.

One question, what happens in this scenario:

Q = require 'q'

asyncMeth = (x, y, next) ->
  next(null, x*y)
  next(null, x*y*2)
  next(new Error('returned error'), x*y*3)
  throw new Error('thrown error')

asyncMethP = Q.nfbind asyncMeth, 5, 2

syncMethP().then(console.log, console.error)

[crito]

Well you could return a promise in cases where next is not applied as an argument. To forget to call a callback in the JavaScript world sounds like a weird reasoning to me.

Your example with nfbind doesn't work like that I think. nfbind replaces a callback that gets called with err as first and result as second argument (node style). Promises can be resolved only once and than they are either fulfilled or rejected and never change anymore. So I believe your example won't work. What you are doing is calling the same continuation multiple times. Not sure if that's the implementation but nfbind probably does something along that line:

nfbind = (func, args...) ->
  deferred = Q.defer()
  next = (err, result) -> if err then deferred.reject(err) else deferred.resolve(result)
  func.apply(null, args.concat(next))
  deferred.promise

[balupton]

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise is some really good docs

[crito]

Yeah, I like race, it's a great function name ...

On Wed, Aug 13, 2014 at 01:52:10PM -0700, Benjamin Arthur Lupton wrote:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise is some really good docs

---

Reply to this email directly or view it on GitHub: https://github.com/bevry/ambi/issues/2#issuecomment-52109137

[christav]

Been a long time since this was posted, do you still want promise support in this thing? If so, it's be pretty trivial.

Assuming there's a global Promise object that corresponds to Promises/A+ spec, a function that returns a promise would look like a sync function (since it returns a value synchronously). Once you have that value, do something like:

    Promise.resolve()
      .then(() => value)
      .then(function onSuccess(v) { callback(null, v); },
        function onError(err) { callback(err); })

This would transparently handle both actually synchronous values and Promises in the same code path.

[balupton]

Fantastic. Let's do it.

[christav]

I'll put together a PR, give me a day or two.

[balupton]

Ok, let me update it to the latest conventions first.

[balupton]

Ok, it's updated to the latest conventions.

[christav]

@balupton PR sent!

[balupton]

Released to v4. Also released v5 which returns a promise instead. Thanks so much for your help on this @christav!