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

koa2 support #329

Closed
jpravetz opened this issue Feb 19, 2017 · 16 comments
Closed

koa2 support #329

jpravetz opened this issue Feb 19, 2017 · 16 comments

Comments

@jpravetz
Copy link

See issue #226
koa-restify-mongoose does not support koa2. I am working on a fork of koa-restify-mongoose at https://github.com/jpravetz/koa-restify-mongoose. I've contacted LucianoGanga, but he hasn't responded. His fork did not have unit tests working. I am working on updating the express-restify-mongoose unit tests to work with koa2. There are sufficient changes that I may publish as koa2-restify-mongoose once it's polished enough. Comments or suggestions welcome.

@Zertz
Copy link
Collaborator

Zertz commented Feb 19, 2017

As long as the integration is seamless, I am open to including koa support.

@jpravetz
Copy link
Author

Express and koa2 support are incompatible and will need it's own repo I believe. I don't see a way to keep them in sync. I want to make sure I am doing the right thing by the community by creating a new repo, and that I am not working cross purposes with anyone else.

@Zertz
Copy link
Collaborator

Zertz commented Feb 19, 2017

I think you are correct, indeed. I would love the core functionality to be decoupled from the frameworks we choose to support but, to be honest, I unfortunately don't have enough spare time to invest anymore.

@jpravetz
Copy link
Author

That is an interesting point, about keeping the core functionality separate. I'll ponder this as I go along, but of course have a day job too :-) I think it's unlikely to be worth it in this case though.

@b-gran
Copy link

b-gran commented Feb 19, 2017

I've put a decent amount of work into such a decoupling in #314. Started a new job recently so haven't had time to finish it, but plan to ramp back up in the next couple of weeks.

The architecture of that branch works by immutably updating a state-machine-like construct each time a middleware is called. It should be possible to just drop it in to koa once complete.

If you feel like working on the decoupling, feel free to submit PRs onto https://github.com/b-gran/express-restify-mongoose/tree/issue-305

It's mostly done at this point. There are really just two tasks left:

It probably wouldn't hurt to clean up the core immutable ERM state and make it a little more generic (it has some vestiges of Request coupling), but it could still be released before that task is finished. Maybe explicitly create a state machine. There are some nice properties of a non-directed ERM state, though.

@Zertz
Copy link
Collaborator

Zertz commented Feb 19, 2017

I somehow momentarily forgot about the great work @b-gran has put toward this idea.

If we only have to maintain a thin layer to support other frameworks, I wouldn't mind having glue code in this repo and peer dependencies on express/restify/koa so users only have to opt-in to their framework of choice.

Basically, we would have a top-level API for custom integrations (for serverless or home-grown tools) and built-in integrations with the popular frameworks.

@jpravetz
Copy link
Author

I'm looking at your @b-gran fork and can see potential. I'm trying to think thru (ctx,next) vs (req,res,next), managing state within the middleware (koa should use ctx.state.erm), and return values (koa returns next() which is a promise). Maybe there's just a koa folder for koa middleware, and all the common functionality is split out. I haven't looked at the unit tests yet and if those can be abstracted in any way. Thanks guys for the leads.

@b-gran
Copy link

b-gran commented Feb 19, 2017

The easiest way to integrate with koa is to start with ERMOperation (https://github.com/b-gran/express-restify-mongoose/blob/issue-305/src/ERMOperation.js).

All you have to do is come up with some way to set the ERMOperation fields using koa middleware. The Express implementation isn't really important -- you can use whatever method you want, as long as the fields get set.
Then, one of the database operations (https://github.com/b-gran/express-restify-mongoose/tree/issue-305/src/api/db) gets called, followed by an output function.
The database op is completely decoupled from a web framework.
The output function will need to be framework-specific, so you'll need to write a koa middleware that takes a fully populated ERMOperation and sends it to the client as JSON.

And as for tests:

  • The unit tests are already decoupled, or at least nearly all of them are. The only ones that might not be are the tests for middleware that is going to be replaced.
  • The integration tests are already framework-agnostic, except for the setup/teardown code. So all you'd need to do is write a setup/teardown function for a koa webserver that mounts ERM. Easy!

@jpravetz
Copy link
Author

jpravetz commented Feb 20, 2017

Happily ERMOperation is what I found to be the starting point too :-). I'd have to modify express-restify-mongoose in minor ways:

  • the app.use of merging ERMOperation
  • call methods get/post/put/... on an object that can be either the app (express) or a router (passed in for koa via options.router) [I think I need to do this, but will confirm when I re-immerse myself in routing]

It's also nice just to have an errorHandler as the first middleware, that handles Promise.reject() and outputs the error, rather than use onError. So I think a new koa folder with duplicate middleware might be best.

@jpravetz
Copy link
Author

jpravetz commented Feb 20, 2017

Okay, I fully agree that @b-gran's fork is the way forward. I've checked in my changes here, with a new koa subfolder for koa middleware. No PR yet - I can generate one if it makes diff easier, but it's not ready yet of course.

There's a top level [option.framework=express] {String} to control if it's koa (should be able to autodetect this from the app parameter, now that I think of it). And an option.router {koa-router} that also needs to be passed in.

I've started to get my head around Transformation. I created a Context class to abstract away koa ctx vs express req/res for use with Transformation. I'm working thru the unit tests (express regressions) making good progress. Not sure what my schedule will be to complete this yet.

@b-gran
Copy link

b-gran commented Feb 22, 2017

Hey @jpravetz thanks for all the work you've done over in https://github.com/jpravetz/express-restify-mongoose/tree/issue-329 -- I left a few comments over there.

It might be helpful to create PR against https://github.com/b-gran/express-restify-mongoose/tree/issue-305 so we can track changes and discuss in one place

@Zertz
Copy link
Collaborator

Zertz commented Feb 22, 2017

Glad you guys are working things out! Let me know if you need anything :)

@jpravetz
Copy link
Author

Update. The koa2 work is done, unit and integration tests are working. All that remains is to verify the work in a real project, which is delayed somewhat by other Real World interruptions, plus koa routing issues that have caused me to waffle between using koa-router and koa-better-router.

@SwenChan
Copy link

Hey @jpravetz thanks for your work, I'm looking for this package now, and how can I use your version for in my koa2 project?Because I can't find your package in npm = ) .

@jpravetz
Copy link
Author

jpravetz commented Apr 15, 2017 via email

@Zertz
Copy link
Collaborator

Zertz commented Jun 11, 2017

I wish I could help but time is a bit limited and I don't have any knowledge of koa, if someone is willing to wrap this up I'd be happy to accept a PR!

@Zertz Zertz closed this as completed Jun 10, 2018
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

4 participants