remove bluebird

[scttcper]

removes bluebird requirement and wraps the JWT verify function in a native promise.

[sdd]

I like this. Bluebird is a large import for simply promisifying a single function that can be done as simply as this PR does. I think i'd like it more if the import of jsonwebtoken and promisification is moved to another file verify.js, which exports the promisified verify function, that index.js imports.

[scttcper]

@sc0ttyd added a verify.js

[sdd]

Looks good, thanks! Just two very minor things. The import of jsonwebtoken in index.js is now unneeded. And, on reflection, I think that the verify.js and verify function would be better as verify-jwt.js and verifyJWT.

[sdd]

@scttcper I've pulled your commits into another branch so that I could resolve merge conflicts with the branch I was working on. Have a look at PR #72 if you get chance

[nfantone]

@sc0ttyd Beware that removing Bluebird is a backwards incompatible change since you are returning the Promise from the exported function (at least in the Koa v2 version). The release bump including this change should be a major one if we are following strict semver.

[scttcper]

@sc0ttyd looks good.

[sdd]

Koa v2 processes it's middleware stack internally through koa-compose, which uses any-promise. Perhaps in verify.js we should be using const Promise = require('any-promise'), so that we use the same Promise implementation as the code that handles the promise that we return?

[nfantone]

Koa v2 processes it's middleware stack internally through koa-compose, which uses any-promise.

You are right on that. Good call.

But what about potential modules that could be extending this middleware functionality by calling the exported function manually? I mean, like so:

const jwt = require('koa-jwt')();

module.exports = function(opts) {
  return jwt(opts)
    .thenReturn(someObject) // Deliberately using Bluebird's API
    .asCallback(done);
}

I understand this scenario is less likely to occur in the wild, but not impossible. And we'll be breaking those.

Perhaps in verify.js we should be using const Promise = require('any-promise').

Unsure, TBH. What are the implications of that? And, if we do it, wouldn't it be against the intention of this PR (removing a dependency on a Promise library)?

[sdd]

And, if we do it, wouldn't it be against the intention of this PR (removing a dependency on a Promise library)?

We would be swapping one dependency for another, yes. But it would be one that is guaranteed to be present anyway since koa-jwt is pretty much always going to be used with koa, and koa transitively depends upon any-promise via koa-compose.

But what about potential modules that could be extending this middleware functionality by calling the exported function manually?

Good example, I didn't consider that. With that in mind, I think you're right regarding the major version bump.

[sdd]

After thinking about it a bit more and looking into it, the SemVer spec itself states as point 1:

Software using Semantic Versioning MUST declare a public API. This API could be declared in the code itself or exist strictly in documentation. However it is done, it should be precise and comprehensive.

Since our documentation and koa's documentation on the usage of middleware are as close as we come to having an explicit public API, we either consider that to be our public API or we aren't SemVer.

With this in mind, usage of koa-jwt that extends the library and depends upon using bluebird-only promise methods rather than using it as documented could be considered to not be using the public API. As such, we would not require a major version bump.

[sdd]

Although this PR shows as merged, the changes were not incorporated due to the ongoing discussion, and were reverted by a subsequent commit. Unfortunately merging PR #72 caused GitHub to mark #58 as merged and will not let me re-open that PR (sorry @scttcper!). Probably best if we continue to discuss on Issue #74