Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow promisifying methods (not just functions) #27

Closed
norswap opened this issue Dec 18, 2016 · 10 comments
Closed

Allow promisifying methods (not just functions) #27

norswap opened this issue Dec 18, 2016 · 10 comments
Labels

Comments

@norswap
Copy link

norswap commented Dec 18, 2016

promisify binds the receiver explicitly, but what if we would like to call the promisified function with different receivers?

As a workaround, I use this function:

function promisify_unbound(f) {
    let flat = promisify(function(self, ...args) { return f.apply(self, args) })
    return function(...args) { return flat(this, ...args) }
}

But maybe there should be built-in support for this?

@mikehall314
Copy link
Owner

You should be able to pass a thisArg as the parameter to promisify. e.g. promisify(o.method, o);

@norswap
Copy link
Author

norswap commented Dec 18, 2016

I know, this is what I meant by "binds the receiver explicitly". Whenever you use the function returned by promisify, this will always be o inside the method.

What I'm doing is something like this:

MyClass.prototype.method_promise = promisify_unbound(MyClass.prototype.method)
...
instance1.method_promise(...) // this = instance1 inside the method
instance2.method_promise(...) // this = instance2 inside the method

@mikehall314
Copy link
Owner

Ah, I see. Do you think this is a good use case for a promisify library? Would it not be simpler to just return a promise?

@norswap
Copy link
Author

norswap commented Dec 19, 2016

I meant to use it on methods that belong to libraries, which I did not write. Seems like a pretty good use case to me. (So MyClass is a bad name -- it should be LibraryClass.)

@mikehall314
Copy link
Owner

I'm unconvinced of the use case for this. If you're modifying the prototype of a library, I'm not sure why you wouldn't do something like:

LibraryClass.prototype.method_promise = function (...args) {
    return new Promise((resolve, reject) => {
        args.push((err, data) => {
            if (err) {
                return reject(err);
            }
            resolve(data);
        });
        this.method(...args);
    });
};

or even:

LibraryClass.prototype.method_promise = function (...args) {
    if (!this._method_promise) {
        this._method_promise = promisify(this.method, this);
    }
    return this._method_promise(...args);
};

But if you want to submit a PR, I'd be happy to merge it :)

@norswap
Copy link
Author

norswap commented Dec 19, 2016

By the same token, you can always promisify a regular function by doing:

fun promisified(...args) {
    return new Promise(function (resolve, reject) {
    libFunction(...args, function(err, data) {
             if (err == undefined) resolve(data)
             else reject(err)
        })
    })
}

Isn't the whole point of the library to do this as a one-liner,
and by the same token, shouldn't it also be a one-liner for methods?

@mikehall314
Copy link
Owner

As I say, if you want to submit a good PR I'd be happy to merge it in.

@norswap
Copy link
Author

norswap commented Dec 19, 2016

I'll do it.

A question regarding the interface however: my preference would be to drop the support for binding (the promisify(o.method, o) syntax). The unbinding way is more general and if you want binding it's very easy:

promisify(o.method).bind(o)
// or equivalently
promisify(o.method.bind(o))

Is that okay?

@zhahaoyu
Copy link

@norswap I think you can just do promisify(o.method).bind(o)

@dgoodua
Copy link

dgoodua commented Jan 4, 2018

https://github.com/digitaldesignlabs/es6-promisify/blob/7eb2f5e9ae858742d495978efebafaee6719da97/lib/promisify.js#L40

Change this line to
let target = this;

This must solve problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants