Promises

[ZachHaber]

I saw this fork from the network graph on github's insights tab, and from the name and the current frequency of commits, it seems that this planning on continuing support/development of passport. I would like to throw in my vote (whatever that counts for) to support promises. Looking forward to seeing this develop further! 😃

[guyellis]

@rwky I haven't looked at the fork that @ZachHaber is referencing. A popular way of doing a non-breaking transition (or dual implementation) of callbacks and promises is to return a promise if a callback is missing. I know that's an oversimplification... might be viable.

@ZachHaber - do you have a link to the fork that has promises - how complete is it?

[ZachHaber]

Sorry if I got the terminology wrong, I was referring to this AKA passport-next/passport (which is a fork of jaredhanson/passport). I suppose it is a proper Javascript issue to fail to bind "this" correctly 😛. Let's just hope this gets the right closure 😉 . I was thinking that the dual implementation makes sense that way this is still capable of being easily substituted instead of jaredhanson. I did have one other question: What are you planning to solve/improve/change from the original passport.js that is the reason this fork exists?

[guyellis]

@ZachHaber the existing jaredhanson repos aren't accepting PRs anymore. This usually happens when a project has been abandoned and the original owners no longer have time to continue work on it.

[rwky]

I love promises and I think integrating them in to passport would be great. It won't be a small job I think we should complete linting #3 first so then the code is at least modern, then we can look at promises.

[guyellis]

@ZachHaber - I agree with @rwky - we should get linting in place before we tackle promises. If you feel like tackling that task and create a PR that adds linting then that will get us one step closer to promises. I've created https://github.com/passport-next/passport/issues/8 to add linting to this project. Feel free to tackle that if you want.

[guyellis]

@rwky another approach we could take with promises is to make it a breaking change to 2.x and drop the callbacks from that major. That would simplify the approach because we wouldn't be supporting 2 styles.

Advantages:

• Simpler codebase which means fewer bugs and less work when making changes which in turn will lead to a higher quality codebase.

Disadvantages:

• No easy/direct migration for existing users to version 2.x.
• Might need to back port fixes to the 1.x series for users still using callbacks and users wanting to migrate to this codebase.

[rwky]

I'd rather not switch to full blow promises yet. I'd probably do something like this:

• Add in promises, deprecate callbacks, update to 2.x
• Give it a while then release 3.x with callbacks removed

That way we only have one version at a time to maintain and people have a while to upgrade (disclaimer: I've some code that's running this module I wouldn't want to upgrade to promises right now).

Once node 6 is EOL (April 2019) I imagine promise support might boost a bit across modules since everyone will then have access to async/await.

[rwky]

Now linting has been done we can look at this :+1: