medikoo / deferred

Modular and fast Promises implementation for JavaScript
ISC License
364 stars 20 forks source link

analagous function to Q.ninvoke #26

Closed ravenscar closed 10 years ago

ravenscar commented 10 years ago

I've recently switched over from Q to deferred in an effort to track down some unresolved promises (easily done thanks!), it mainly went smoothly but there remain a few Q calls for which I can't find an alternative in deferred for.

I would like to use something like promisify but on a method of an object rather than a function. In Q this would be Q.ninvoke(foo, "bar", args...) but I can't see how I would do this easily in deferred.

I could do: deferred.promisify(foo.bar).bind(foo)(args...).then(... but I'm not sure if it has any side effects.

Similarly I could do foo.promiseBar = deferred.promisify(foo.bar) foo.promiseBar(args...).then(... but that seems overkill.

In any case it seems it would be nice to have a version of promisify that deals with methods simply to have more readable code.

medikoo commented 10 years ago

Hi @ravenscar

Indeed deferred provides just promisify for that. The reason is to encourage creation of reusable wrappers, as in most cases it's not about one time call, and reusable wrappers make final algorithm way cleaner (no Q.invoke or deferred.promisify noise)

Best way to deal with methods, is to attach promise version on prototype, so:

Foo.prototype.pBar = deferred.promisify(Foo.prototype.bar); // One time wrapper

foo1 = new Foo();
foo2 = new Foo();

// Promise version accessible on any instance:
foo1.pBar().done();
foo2.pBar().done();

Other way is to do as you suggested inline: deferred.promisify(foo.bar).call(foo, args...) it will work perfectly fine, no side effects, but that's poor choice for readability, and not that efficient as per each call new function is created.

Methods of which module you try to promisify? Is this some popular public one?

Let me know if this is helpful. I can add Q.invoke equivalent, but first I want to be sure, there's really need for that..

ravenscar commented 10 years ago

I first encountered it with the sqlite3 module but since I was using it often I wrote some wrappers for the functions I needed so that's fine now.

In this instance I'm using the session.socket.io library with express. This ends up passing me a Session at some point which I need to call save() on in a few instances.

Now I do expect a session object here, or at least an object with the same interface as the session, but I wouldn't like to rely on the session.socket.io library always passing an instance of a particular class here. It should be fine for the third-party module to subclass Session, or use some kind of proxy, and I shouldn't really make guesses about how it works internally and make assumptions about the object it passes beyond the interface.

That leave me doing something like the following:

if (!session.promiseSave) {
  Object.getPrototypeOf(session).promiseSave = deferred.promisify(session.save);
}
session.promiseSave(args...).then(...

Internally I build a lot of objects using mixins rather than inheritance. A persist() method is often one of the first methods to be mixed in and could use a memory store, database, or network storage. In this case the above method would not work properly as the persist method itself varies from object to object despite those objects having the same prototype. I realise that is somewhat academic and since I have access to the internals I'm using deferreds directly in this case.

I guess I am trying to say that in the case that a third party module returns an object which I know to have a method with node calling style I don't think it's necessarily a good idea to augment the prototype, and it may be best to use a pattern which makes the least assumptions.

Having said that prior to using deferred I was using Q.ninvoke all over the place so there's definitely a good case to not include that functionality in deferred.

medikoo commented 10 years ago

@ravenscar thanks for thorough explanation, all your points are very valid. I gave it a thought, and it's difficult not to agree that augmenting prototypes of imported modules is not best idea.

Therefore I've introduced callAsync(fn, context, ...args) -> https://github.com/medikoo/deferred#callasyncfn-context-args

It slightly differs from Q.ninvoke, and it may feel weird if you got used to its arguments order. However I feel it's more logical to treat function as a key object (that's taken as first argument) and make rest of signature to resemble Function.prototype.call.

Published as v0.6.7

medikoo commented 10 years ago

I've just realized that callAsync in case of methods will force you into repetitive writing e.g. obj.method, obj, therefore I've decided to introduce also function version of invokeAsync promise extension, (internally it stands on callAsync same as promisify now does). It has same signature as Q.invoke.

Published as v0.6.8