search

sb8244 / passwordless-hapi

very thin adapter over top of the passwordless express middleware
8 stars 0 forks source link

Update module to work with hapi v17+ #1

Open franvarney opened 6 years ago

franvarney commented 6 years ago

Hapi 17+ no longer uses callbacks and has otherwise changed quite a bit so the current version of this module will not work with any current/future versions of hapi.

I propose not writing it as an adapter any longer either. In Franksteining this code to work with hapi v17, I found error handling and debugging to be a pain. Additionally, I believe would be easier to add some more options/customization if it was not relying on the express module.

sb8244 commented 6 years ago

Hey Fran, is this something you'd be interested in tackling? The tie into express modules is because that is what passwordless is written in. Unless HAPI is compatible with express modules, I can't foresee it not being an adapter.

franvarney commented 6 years ago

I'm interested, but it might take a bit. And I would definitely like your (or community) feedback or other input!

I don't think an updated version needs to depend on the existing passwordless module. When I pieced together my variation of it, I rewrote a couple of the methods and used util.promisify. That's an option to move forward with, but I don't think the code looks very nice at all and as mentioned before, errors were getting lost. It could be better to just reference the passwordless module for the overall functionality instead..?

I think the biggest downside to moving away from the adapter pattern currently is that the token store modules seem to use callbacks still. So there will either be callbacks mixed with promises or a few util.promisify.

Feel free to email me if you want to voice chat about this at all since it's faster.