Extend Router.routes rather than overwriting #2255

Closed
BigstickCarpet opened this Issue Feb 10, 2013 · 4 comments

Comments

Projects
None yet
3 participants

Let's say I extend the Router class and define some generic routes that I'll always want:

var CustomRouter = Backbone.Router.extend({
    routes: {
        "help":                 "help",    // #help
        "search/:query":        "search",  // #search/kiwis
        "search/:query/p:page": "search"   // #search/kiwis/p7
    }
});

And then I create an instance of my CustomRouter class, but pass a few page-specific routes to the constructor:

var router = new CustomRouter({
    routes: {
        "tab:tabNumber":                 "loadTab",   // #tab2
        "sort/:sortBy":                  "sortTable"  // #sort/firstName
    }
});

The problem is that the Router constructor completely overwrites the default routes property with the new routes that were passed to the constructor. I think the two lists of routes should be merged instead. This would require changing the following line of code in the Router constructor:

if (options.routes) this.routes = options.routes;

...to this instead:

if (options.routes) this.routes = _extend(this.routes || {}, options.routes);

What do you think?

Collaborator

akre54 commented Feb 10, 2013

Similar things have been proposed (most specifically for View's events hash) (see #1816, #244, and 6bb43c1)

The general consensus was that Javascript inheritance doesn't work this way (i.e. your Router shouldn't be automagically merging the two hashes by default.) If you pass in routes via the options hash, this should override your existing this.routes.

An alternative solution would be for Router to allow routes to be defined as a function (the way a lot of Model and View properties are) in _bindRoutes, and then called with _.result like this:

RouterA = Backbone.Router.extend
  routes: ->
    "help":                     "help"    # help
    "search/:query":            "search"  # search/kiwis
    "search/:query/p:page":     "search"   # search/kiwis/p7

RouterB = RouterA.extend
  routes: ->
    _.extend super,
      "tab:tabNumber":           "loadTab"   # tab2
      "sort/:sortBy":            "sortTable"  # sort/firstName

r1 = new RouterA
console.log _.keys r1.routes # ['help', 'search/:query', 'search/:query/p:page']

r2 = new RouterB
console.log _.keys r1.routes # ['help', 'search/:query', 'search/:query/p:page', 'tab:tabNumber', 'sort/:sortBy']

r3 = new RouterB
  routes:
    'foo': 'bar'
console.log _.keys r3.routes # ['foo'] (passing in routes overrides no matter what)

That would be an acceptable solution as well. Are there any plans to do that?

Collaborator

akre54 commented Feb 10, 2013

not until you open a pull 😉

davidinjc pushed a commit to davidinjc/backbone that referenced this issue Mar 15, 2013

davidinjc pushed a commit to davidinjc/backbone that referenced this issue Mar 15, 2013

Owner

jashkenas commented Mar 19, 2013

Merged #2375.

@jashkenas jashkenas closed this Mar 19, 2013

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