-
Notifications
You must be signed in to change notification settings - Fork 81
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
Allow async handlers #138
base: master
Are you sure you want to change the base?
Allow async handlers #138
Conversation
FYI, express-openapi supports this. |
Thanks, I will look into that lib. |
Is there a reason not to accept this PR though ? express-openapi does not work out of the box for me, I'd rather stick with swaggerize-express |
@juliusza what issue were you having? Can you open one? https://github.com/kogosoftwarellc/express-openapi/issues |
ok, how about this one? |
Will review. Kraken team has been under a lot of resource constraints and it’s been hard to turn around PRs. Apologies. |
Can you add some tests? |
Also, for what it’s worth at this point https://github.com/krakenjs/hapi-openapi is more actively maintained. Unless you have some overriding preference for express applications. |
I hope test is satisfactory. Please also bump major version to 5.0.0, because this now requires native Promise support and might not work for very old nodejs versions. Tests now require node v8 to run. But node 4.8.7 is enough for lib to work correctly. |
Also do you think you could bump enjoi version ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have much of an issue with this. @subeeshcbabu ?
Allows route handlers to be async functions
Based on https://medium.com/@Abazhenov/using-async-await-in-express-with-node-8-b8af872c0016
Or see README for more info