Allow Connect's bodyParser to be configurable

[sylvaingi]

The bodyParser middleware is enabled, but there was no way to configure it, especially the multipart handler that uses formidable under the hood.

This PR adds Meteor.Router.start() that boots up server side routing, and forwards options to bodyParser.

IMHO it is also useful to start server-side routing manually, as it could conflict with other packages meddling with Connect.

[tmeasday]

Hi @sylvaingi

Thanks for your contribution -- I've added you as a collaborator on the repo.

I'm just hesitating on merging this patch as I was considering changing the API for server-side stuff--namely I wanted to rename Meteor.Router.add to something else (maybe Meteor.Router.serve ?) as I realised that in the long term client side routes may also want to be defined server side too.

So if we are changing the SS API we may as well do both at the same time. Any thoughts on the name?

One other thing: Is there a way to retain the present functionality of not having to call Meteor.Router.start() on the server side?

Here are a couple of suggestions:

1. The first Meteor.Router.serve() call triggers start() if it hasn't already been called.
2. We call start() in a Meteor.startup() block and allow people to set options for that call before hand.

I'd just prefer to not make the experience more complex if we don't have to.

[sylvaingi]

Hi there!

I guess the main influence of this PR comes from Backbone, where you have to explicitly call Backbone.history.start() whenever you are ready to route through your app, like after your routers are initialized.

I don't know about page.js internals, but I think it would be also great to be able to start routing at a very specific point in the app lifecycle.

Let me give you an example: I'm currently developing an app (visible here: drumnchat.meteor.com) that allows people to share songs in many "rooms". Basically there are two pages, the room list and a room, each one mapped to a specific route.

I ran into some issues when I tried to access to a room by URL directly, as in my first iteration I added some code to check the room existence in the Meteor.Collection into the route handler. But since the collection data is available after the route handler is called, the existence test would always fail and the redirection code runs. I eventually managed to put "this redirect if the room does not exists" code in another place but the result feels quite clunky.

I hope you got my point! So in my opinion, if you want to enforce API consistency on both sides, the best would be to require a start() call as it gives to devs more control on the initial routing.

[tmeasday]

Hey.

Well I guess the thing is I'd like the default behaviour to be not require a user to call start(), as that always kind of felt clunky and awkward to me. It's not that it's not supported by page.js.

I can accept that there will be use cases where you need what you are talking about, but I'm not sure this is it. Can't you do something like this:

var roomsHandler = Meteor.subscribe('rooms');

Meteor.router.add({
  'rooms/:id': function(id) {
    if (! roomsHandler.ready()) {
      return 'loading';
    }

    var room = Rooms.findOne(id);

    if (! room) // redirect

    // do whatever..
    return 'room';
  }
});

Am I missing something / is this more complex than your current code? I'm not pretending to be an expert on all types of routing, but that's how I'd do it.

Do you want to pull the bodyParser code out into a separate PR (or just a branch) so we can focus on the start() stuff in this PR? Here's what I think you should do:

• Add Meteor.Router.config() that takes the bodyParser stuff as a config
• move the connect "start" stuff into Meteor.startup so people can be sure to call config() before it happens.

Does that make sense?

[sylvaingi]

Hey, thanks for pointing me out the ready() method of the subscriptions, it was added in 0.5.7 but I missed it.

It almost completes my use case, with one quirk left, it seems that redirecting within a route handler confuses the router since end up calling a route handler within another, inception style :)

Looks like the start method is not as useful as it seemed, I'm going to rework this PR to add the config() method.

[sylvaingi]

Snap!

It looks like if we put additional middlewares within a Meteor.startup callback, they end up at the very bottom of the stack, beyond the last middleware and thus are not invoked.

I am open to ideas here!

[tmeasday]

1. Redirecting is a bit tricky, you can do it, you just need to wrap in a defer block:

Meteor.defer(function() { Meteor.Router.to(..); });

I'm working on getting a better method of redirecting at some point, not sure exactly when. (If you'd like to take a look at it, that'd be awesome. The API I'm thinking of is here: https://github.com/tmeasday/meteor-router/wiki/Meteor.Router-v3-API )

I'm releasing the named routes part of that in the next 24 hours but the rest is going to have to wait a little while unfortunately.

1. Damn!

So I guess in your original testing, you were calling Router.start outside of a startup block, in application code.

I really don't want to force people to call Router.start(), but I don't know of a way to get package code to run after application code, barring using a timeout or Meteor.startup() (I'm guessing the first wouldn't work either).

The only other thing I can think of is to make Router.add() start the router if it hasn't already and just make sure people know that they need to call Router.configure() before add().

Or, perhaps we make people call start() on the server only.

Hmmm...

[sylvaingi]

I went with your suggestion to hook _start() on the first call to add(), works like a charm!

One thing is bothering me now, how am I supposed to run the tests? Last time I tried, the only thing working was copying the package folder to Meteor's soruce packages folder and run meteorite from there.

[tmeasday]

If you are on 0.6.0, you can run mrt test-packages ../router/.

If you are on <0.6.0, you can run mrt.

Great stuff by the way, looks perfect!

Let me know when the tests pass, maybe add something to the README, give me the go-ahead and I'll release a new version.

Of course the next thing to do is to make named routes (just release 0.5.0 with them) work on the server (both for client-side and server-side routes) :)