Remove dependency on iron:router

[stubailo]

Not all apps use iron router, and it's kind of janky that you might need to disable the client part sometimes.

I've built a super ultra simple server-side JSON router here: https://github.com/stubailo/meteor-rest/tree/master/packages/json-routes

I would definitely be willing to modify it to accept the response as a return value, like you are using in this library.

Let me know if this is something you would be interested in merging.

[stubailo]

Upon inspecting the issues, looks like this might fix #24 #3

I will also need to make sure my package supports #32 and related.

[kahmali]

I completely agree with moving away from iron router. This looks like a very promising solution. Very close to the metal on connect, with a super simple API.

As far as modifying your package to accept the response as a return value, that probably wouldn't be necessary. Iron router only provided access to the Node response object, just as you do. Restivus wraps endpoints in the Router.route method (the equivalent of your JsonRoutes.add method), and takes care of sending the endpoint return value as the response body using the Node response object (optionally, endpoints can return an object that includes a statusCode, headers, and body).

If you were going to modify anything, it should probably be the JsonRoutes.sendResult method, so that it accepts a headers object, and supports all the common content types (#32), instead of just JSON (of course, this probably warrants a name change for the package). Restivus will still have to wrap responses to provide CORS support and apply some configured settings, but it can't hurt to have a convenience method for sending the response.

I'm also not completely against continuing to use the Node response object for sending the responses from Restivus, since the functionality is already implemented and it's only wrapping a few lines of code to write and end the response. Of course, if you plan to add support for the common response content types (#32), then that would make the sendResult method usable in Restivus.

I agree that this will definitely resolve #24, and probably #3 as well. I'm not sure, but there's a chance this also fixes #35, which you may have some insight on.

Does your package parse URL query params? Restivus relies on iron router's url parsing right now, so we would need to replace that functionality before removing the iron router dependency.

Finally, this is definitely something I would be interested in merging in, as long as it passed my very basic suite of tests (and a few others I would like to add). Thanks for looking into this (and for all your contributions to Meteor in general)!!

[stubailo]

@kahmali I'm getting these test failures on dev, is that expected? https://www.dropbox.com/s/xlzuet808a1cbpv/Screenshot%202015-04-27%2012.24.56.png?dl=0

Almost done with an initial PR that passes the tests, we just need to add more tests to make sure the query parsing and URL parsing matches what you had before with iron router.

[kahmali]

Yes, those failures are expected. I should just comment out those tests until I can fix them. The status code changes are API-breaking (for API consumers), so I've held off on fixing them until the next major update. I should have at least left a comment with an explanation. Sorry about that!

I'm very excited to move away from Iron Router! Let me know if there's anything I can do to help. I have a few tests on an old branch with some query params test, and I'll add some more asap.

[stubailo]

@kahmali Right now, I'm trying to make a PR with no API changes at all, simulating Iron Router jankiness where necessary. I think there are definitely some API improvements that would be nice (for example, returning JSON on an error instead of plain HTML text), but I'm not sure what to do about backwards compatibility.

Going to submit the initial PR now, but it's a work in progress for sure until we can add more tests to cover more edge cases.

[kahmali]

Resolved by #46.