Router calls the wrong method #996

Closed
jockster opened this Issue Feb 13, 2012 · 12 comments

Projects

None yet

5 participants

@jockster

Perhaps I am just doing something wrong, but I think I've found a bug in the way backbone.js handles URI routing.

It appears that the behaviour of how Backbone handles URI's changes when I enter an URI into the browser compared to when I manually force the URI by invoking myRouter.navigate().

Accessing all URI´s - /contacts, /contacts/new and /contacts/ID works well in the browser, but triggering /contacts/new manually fails as it instead calls the route method assigned to the /contacts/ID URI (loadContact).

To the code:

myApp.contactRouter = Backbone.Router.extend({

    routes : {
        "contacts": "contacts",
        "contacts/new": "newContact",
        "contacts/:id": "loadContact"
    },

    contacts: function() { console.log("in contacts") },

    newContact: function() { console.log("Creating new contact"); },

    loadContact: function(id) { console.log("Loading specific contact with id ", +id); }
});

And here's my failing jasmine.js-based unit test of the newContact method:

it("Opens /contacts/new" function() {

    this.router.bind("route:newContact", this.routeSpy);
    this.router.navigate("contacts/new", true);

    this.router.bind("route:loadContact", function(route) { console.log(arguments) } ); // For debugging - loadContact is called instead!

    expect(this.routeSpy.calledOnce).toBeTruthy(); // Fails
    expect(this.routeSpy.calledWith("new")).toBeTruthy(); // Fails
}); 
@jashkenas
Owner

The behavior is correct. You're allowed to override more general routes with more specific ones that should take priority. Reverse the order in which you add contacts/new and contacts/:id, and you should be good to go.

@jashkenas jashkenas closed this Feb 13, 2012
@jockster

Hi Jashkenas,

Yep. I realized that and tested reversing the order before writing here, but still can't get it working regardless of the route order.

Is there something spooky going on here?

@jashkenas
Owner

Nothing spooky, but it may be in the reverse order from what you expect. I have to correct myself -- more specific routes go at the top.

http://backbonejs.org/docs/backbone.html#section-90

"handers" is just an array -- the first one that matches wins.

@jockster

Hi again,

Ok, but what about the fact that it works great in my browser regardless of route order but not when I manually invoke it using .navigate()?

Same occurs on both 0.9.0 & 0.9.1

@jashkenas
Owner

Huh. Let's try to add a test case to the test suite -- reopening as a bug.

@jashkenas jashkenas reopened this Feb 13, 2012
@wookiehangover wookiehangover pushed a commit to wookiehangover/backbone that referenced this issue Feb 15, 2012
Sam Breed adding test coverage for route precedence as per issue #996 8d8a359
@wookiehangover
Collaborator

@jashkenas I've added some test coverage for this, let me know if that looks like what you were looking for. (Ignore that first commit, wookiehangover@8d8a359 is the one to look at)

@theindustry seems like the only way to make this test case fail is to re-order the routes so the less-specific route is first, like this:

"contacts/:id": "loadContact",
"contacts/new": "newContact"

Perhaps this is an issue with binding to the route events like in your test case?

@jashkenas
Owner

seems like the only way to make this test case fail is
to re-order the routes so the less-specific route is first...

If that's the case, then there's no bug here. This is how it's supposed to work.

@jashkenas jashkenas closed this Feb 15, 2012
@wookiehangover
Collaborator

@jashkenas yep, seems to work as expected. Is it worth it to pull req this additional coverage in?

@jashkenas
Owner

If you'd like ... but it's just testing that iterating over an array in order ... iterates over an array in order.

@wookiehangover
Collaborator

testing that iterating over an array in order ... iterates over an array in order

true >_<

BUT I think that this is useful for coverage any future changes to route precedence, and now there's a demonstration in the test suite of how route order is intended to work.

@benanhalt

testing that iterating over an array in order ... iterates over an array in order

Except that the routes are being defined in an object literal, not an array. What assurance is there that the keys come back in the order they were put in?

See for instance:
https://code.google.com/p/chromium/issues/detail?id=37404

@tgriesser
Collaborator

@benanhalt - in practice (e.g. all major browsers), they are kept in the order they are defined, unless the keys are numeric, which wouldn't be the case with routes. See #1678 for more info.

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