Spine.Route Proposal: Match multiple routes / ability to nest stacks

[adambiggs]

In response to #140, #159, #312, #373, and #398

The Problem

Currently, Spine.Route only executes callbacks for the first matching route it finds. Other matching routes defined later on will never respond.

This means only a single controller can ever listen to a route (without returning false from a route callback as described in #312 [undocumented], or resorting to less flexible workarounds).

The most common problem this presents is that Spine.Stack instances can't be nested.

Proposed Solution

In this pull request, Spine.Route will execute callbacks for all matching routes, starting from the least-specific and moving to the most specific (reverse order they were defined, so that more specific controllers are activated last).

Then the 'change' event is triggered with an array of matching routes as the first argument (instead of a single instance). Routes in the array are in the order they were called.

Example

class MainStack extends Spine.Stack

  controllers:
    'dashboard': Dashboard
    'photos': Photos

  routes:
    '/photos*glob': 'photos' # globs can be used to activate parent controllers
    '/': 'dashboard'


class Photos extends Spine.Stack

  controllers:
    'index': PhotoList
    'view': ViewPhoto
    'edit': EditPhoto

  routes:
    '/photos/:id/edit': EditPhoto
    '/photos/new': NewPhoto
    '/photos/:id!new': ViewPhoto # `!` can be used to exclude specific params
    '/photos': PhotoList

Previously if the URL changed from / to /photos/123/edit, the EditPhoto controller would be activated, but the Photos stack wouldn't... So the user wouldn't see anything change.

With this PR, the Photos stack would be activated first (because the glob route is a match), followed by the EditPhoto controller, resulting in the expected behaviour.

[aeischeid]

I think this looks pretty good. Not something I see myself utilizing much, the nested stacks thing just never popped up as a need for me yet or it was relatively easy to architect around maybe. anyway, I say

[adambiggs]

It's true that there are workarounds, but they aren't documented or intuitive... The #312 workaround also introduces a degree of coupling between controllers.

My goal for this is that nesting controllers, stacks, etc should "just work" 😃

How potentially breaking do you think these changes are? I had to refactor one piece of code in our app that relied on a single route in the 'change' callback...

[aeischeid]

how potentially breaking? hard to say, fairly certain it wouldn't impact any projects I have going on currently, so I say go for it ;)

I'd say we bring this in then work to get #482 in line after that.

[adambiggs]

I found a pretty big breaking change...

routes:
  '/photos/new': NewPhoto
  '/photos/:id': ViewPhoto
  '/photos': PhotoList

Navigating to /photos/new matches the '/photos/:id' route, with { id: 'new' } as the params object...

I guess most people should have some kind of user friendly "record not found" page to catch bad IDs... but still, ViewPhoto is being activated for no reason before NewPhoto which is inefficient...

One possible solution might be some kind of exclude syntax:

routes:
  '/photos/new': NewPhoto
  '/photos/:id!new': ViewPhoto
  '/photos': PhotoList

Then '/photos/:id' would match unless the id param is 'new'. /some-model/new is the only scenario I can think of where you'd really need this.

Any thoughts?

[aeischeid]

Good catch!

We have done something like

'/model/search/:query': Search
'/model/create': Create
'/model/:id/show': Edit

and I can imagine other scenarios that would be affected

[adambiggs]

Added ! syntax for excluding params. Regexp stuff made my brain hurt!

@aeischeid Like you mentioned, we should probably bump up to v1.2 if this is merged since many people will need some code refactoring for the param excludes.

I'll update the route docs if this is merged and we should probably also have a "v1.2 upgrade guide" somewhere.

[jcarlson]

Here's a silly question... rather than building complex routing logic into Spine, have you thought about extracting it out of Spine?

Just like Spine doesn't deal with view technology at all, perhaps it shouldn't mess with routes either and defer such functionality to other existing routing engines, like Crossroads.

Spine could go so far as to recommend an engine and work well with it, but allow the user to implement whatever they want for routing, if they need it at all.

[adambiggs]

@jcarlson Not a silly question at all! I totally agree about not adding complexity to Spine's API. To be honest I don't like the ! workaround this PR introduces... It was an after-thought, and definitely makes routing less intuitive.

Thanks for the link to Crossroads. It looks very powerful, but I think Spine should always include a routing module since it's such a common pattern when building apps with Spine.

This might be just what we need (Backbone has similar).

Instead of Spine.Route matching multiple routes, we could have multiple Spine.Router instances that would each match a single route. Then Spine.Stack instances could create their own Spine.Router instances to match a single route within the stack.

A Spine.Router instance pattern might also help deal with stuff like #479.

Thoughts?

[jcarlson]

I like the multiple instance pattern much better than trying to hack a singleton router. It's so simple! Multiple routers? Yes, please. That really helps keep concerns separated if each stack can manage its own routes.

Maybe all we need is a Router Manager that can hook into the browser's native history events and delegate to each Router instance; a router registry of sorts... or maybe we don't even need that if a Router instance can hook itself up as a native event listener for history changes.

[adambiggs]

Maybe all we need is a Router Manager that can hook into the browser's native history events and delegate to each Router instance; a router registry of sorts...

This could probably be handled at the class level by calling Spine.Router.setup() (pretty much like it works right now). We would just need to split Spine.Route into separate Route & Router classes.

Both new classes could have .release() instance methods to clean up after themselves just like model instances.

[adambiggs]

I'm going to rework this and open a new PR. Thanks for the input @jcarlson!