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

Add support for promises on req.login. #243

Closed
wants to merge 1 commit into from

Conversation

dignifiedquire
Copy link

In response to #227 this uses promise.

Closes #225.

@topaxi
Copy link

topaxi commented Mar 6, 2015

Any news on this?

@listepo
Copy link

listepo commented Jul 16, 2015

Ping

@jaredhanson
Copy link
Owner

On further consideration, I don't have any desire for Passport to support promise-style calling conventions. If need be, it is very easy to promisify req.login specifically in applications that need it.

There's too many opinions and options about how async functions should be called in JavaScript. If it's promises now, it'll be some ES* feature next. I'll follow the lead of Node core for what they expose on http.ServerResponse. If they do promises, Passport will, but not until then.

@jaredhanson jaredhanson mentioned this pull request Jul 16, 2015
@callumacrae
Copy link

@listepo
Copy link

listepo commented Jul 17, 2015

Express.js 5, Promises in all handlers expressjs/express#2259

@jaredhanson
Copy link
Owner

When Express 5 implements support for promises, I'll follow suit for Passport's middleware. Additional calling styles to http.ServerRequest will follow what Node core does with their API.

@Nicholaiii
Copy link

expressjs/express#2237

@nhooey
Copy link

nhooey commented Feb 24, 2017

@jaredhanson How can you promisify req.logIn globally?

I have successfully promisified req.logIn in each function that uses it inside an Express.js app, but haven't been able to promisify it globally:

const Promise = require('bluebird');

exports.whatever = async (req, res, next) => {
    req.login = Promise.promisify(req.logIn);

    // ... snip ...

    try {
        await req.logIn(user);
    } catch (e) {
        console.log(`Error logging in user: ${user.id}: ${e}`);
    }
}

Promisifying passport.authenticate emits this error:

TypeError: app.use() requires middleware functions

At this line in app.js:

app.use(passport.session());

@nhooey
Copy link

nhooey commented Feb 26, 2017

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

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

Successfully merging this pull request may close these issues.

Promises
7 participants