Passport: Promises

[callumacrae]

I have the following code:

    user.save()
        .then(function () {
            req.login(user, function (err) {
                if (err) {
                    next(err);
                } else {
                    auth.login(req, res);
                }
            });
        })
        .error(function (err) {
            winston.err(err);
        });

It would be nicer as:

    user.save()
        .then(function () {
            return req.login(user);
        })
        .then(function () {
            auth.login(req, res);
        })
        .error(function (err) {
            winston.err(err);
        });

If I added support for promises to passport, would you merge it?

[jaredhanson]

Would this only impact req.login?

Sent from my iPhone

On Apr 7, 2014, at 1:20 AM, Callum Macrae [email protected] wrote:

I have the following code:

user.save()
    .then(function () {
        req.login(user, function (err) {
            if (err) {
                next(err);
            } else {
                auth.login(req, res);
            }
        });
    })
    .error(function (err) {
        winston.err(err);
    });

It would be nicer as:

user.save()
    .then(function () {
        return req.login(user);
    })
    .then(function () {
        auth.login(req, res);
    })
    .error(function (err) {
        winston.err(err);
    });

If I added support for promises to passport, would you merge it?

—
Reply to this email directly or view it on GitHub.

[callumacrae]

I think so; do any other functions accept callbacks?

On 7 Apr 2014, at 15:20, Jared Hanson [email protected] wrote:

Would this only impact req.login?

Sent from my iPhone

On Apr 7, 2014, at 1:20 AM, Callum Macrae [email protected] wrote:

I have the following code:

user.save()
.then(function () {
req.login(user, function (err) {
if (err) {
next(err);
} else {
auth.login(req, res);
}
});
})
.error(function (err) {
winston.err(err);
});
It would be nicer as:

user.save()
.then(function () {
return req.login(user);
})
.then(function () {
auth.login(req, res);
})
.error(function (err) {
winston.err(err);
});
If I added support for promises to passport, would you merge it?

—
Reply to this email directly or view it on GitHub.
—
Reply to this email directly or view it on GitHub.

[jaredhanson]

No, that should be it. I'm not opposed. If you need to pull in a promise library, I'd prefer promise since that seems the most lightweight.

[jaredhanson]

Closing. See comment for rationale.

[nhooey]

See issue #536 for adding promise support now that a lot of time has passed since this issue was created.