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

All JWT-related functions should be yieldable #32

Closed
vsviridov opened this issue Dec 24, 2015 · 14 comments
Closed

All JWT-related functions should be yieldable #32

vsviridov opened this issue Dec 24, 2015 · 14 comments

Comments

@vsviridov
Copy link

not just verify

@tejasmanohar
Copy link
Member

Personally, I don't think it's the responsibility of this middleware to expose such methods, as the client can use the jsonwebtoken module directly and get choice over what version of the module they use, what methods they use, and whatnot. the module provides both sync + async (callback) variants of each method and is easy to promisify (with or without a module helper) :)

Thoughts, @stiang?

@stiang
Copy link
Collaborator

stiang commented Jan 19, 2016

@tejasmanohar I think I agree. In retrospect, it seems like a mistake to "forward" these methods, since it goes against the node philosophy of having small modules that do their one thing well. If you need these methods, you should require jsonwebtoken in your code.

@tejasmanohar
Copy link
Member

Yep, agreed. Will look into it and send PRs either today/tomorrow evening. Maybe we can bump a version with some of the existing PRs and these changes soon :)

@stiang
Copy link
Collaborator

stiang commented Jan 19, 2016

That would be awesome! We’ll need to bump the version quite a bit, though, since removing the jsonwebtoken functions is likely to break existing code.

@tejasmanohar
Copy link
Member

Yep, makes sense to merge stuff into a v2 branch and prepare for that release- following semver.

@leo
Copy link

leo commented Apr 21, 2016

If you ask me, we should start thinking about making it working with await within async functions instead of using generators and yield (because that is how the latest version of Koa works).

@fl0w fl0w mentioned this issue Apr 21, 2016
@vsviridov
Copy link
Author

@leo Koa authors state that they will maintain 1.x branch up until the async/await spec is fully available natively. And it might be a while, considering that they just added things like ES6 destructuring to Node...

@leo
Copy link

leo commented Apr 22, 2016

@vsviridov Yeah, that's true! However, I see many devs that are using Babel to transpile their code before running it.... 😊

@vsviridov
Copy link
Author

@leo, yeah, i guess. But I feel that it's sooo clunky on server-side...

@nfantone
Copy link
Member

nfantone commented Jun 1, 2016

@stiang I believe this should be closed. jsonwebtoken functions are no longer begin "forwarded".

@fancyoung
Copy link

Does it mean we must require('jsonwebtoken') to generate token when login?

@tejasmanohar
Copy link
Member

@fancyoung
Copy link

@tejasmanohar yes, but it seems sign function not yield in v2, only verify works.
https://github.com/koajs/jwt/blob/koa-v2/index.js#L39

@nfantone
Copy link
Member

@fancyoung Be aware that if you install koa2 version (i.e.: npm i koa-jwt@koa2), jsonwebtoken methods are no longer exported. They were never really needed in the first place.

Also, this issue really needs to be closed.

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

No branches or pull requests

7 participants