Route dispatching needs to be smarter about parameters #283

polotek opened this Issue Dec 12, 2012 · 3 comments

3 participants


First the problem. I've autogenerated routes for my controller with router.resource('posts'). Then I want to add some custom actions to the controller. But the dispatcher doesn't reach those routes because it gets stopped too soon by the show route. Like so.

// controller/posts.js

this.index = function() { ... } = function() { ... }
this.compose = function() { ... }

// routes.js

If you run the server and hit http://localhost:4000/posts/compose in your browser, you get an error.

TypeError: Cannot call method 'toObj' of undefined
    at /Users/polotek/src/writedown/app/controllers/posts.js:40:48
    at utils.mixin.load (/usr/local/lib/node_modules/geddy/node_modules/model/lib/adapters/memory/index.js:57:7)
    at Object.obj.all (/usr/local/lib/node_modules/geddy/node_modules/model/lib/index.js:407:25)
    at Function.obj.first (/usr/local/lib/node_modules/geddy/node_modules/model/lib/index.js:387:18)
    at show (/Users/polotek/src/writedown/app/controllers/posts.js:39:22)
    at callback (/usr/local/lib/node_modules/geddy/lib/controller/base_controller.js:347:22)
    at controller.BaseController._handleAction (/usr/local/lib/node_modules/geddy/lib/controller/base_controller.js:360:7)
    at finish (/usr/local/lib/node_modules/geddy/lib/app.js:376:34)
    at /usr/local/lib/node_modules/geddy/lib/app.js:559:19
    at localCallback (/usr/local/lib/node_modules/geddy/lib/sessions/index.js:

This is because the url was matched by the route /posts/:id.:format which was added by routes.resource. The problem is that it's not an id, it's a controller action. The easy fix is to put any custom routes before the call to router.resource. But I believe other frameworks like rails handle this issue. If the show controller tries to look up a post with the given id and fails, it should bubble back up to the route dispatcher and continue.

I could be wrong because I don't know a ton about rails. And our job would get more difficult because it would make route dispatching asynchronous. I'd rather avoid that. But I also feel like this problem is one that a lot of people are going to run into.


We could accomplish this by calling something like:

// grab all the matching routes
var params_array = router.all( req.url, req.method )

var success = true
do {
  // pull the first match of the array
  var params = params_array.shift()
  // attempt dispatch
  success = fake_dispatch( params.controller, params.action, params )
} while (!success) // retry if the dispatch fails

This isn't very node-y, but you could pass in a next() method that does much the same thing. You get the idea.

Personally, I think this is a terrible idea. You can pre-vet the routes using match conditions so you never have to reject something in the controller.

For more complicated situations, there's also deferred routes, which I still haven't bothered writing tests or docs for. Sorry :-/


For the record, there's also (a probably undocumented) feature to nest routes under resources:

// only under the member (with :id)

// only under the collection (index)

// or either (both?)

// routes work too (with nest)

(tests are here for examples)

My poor documentation practices, in a nutshell:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment