Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an optional argument 'trailingSlashIsSignificant' to Backbone.Router... #3136

Closed
wants to merge 1 commit into from

Conversation

aholmes
Copy link

@aholmes aholmes commented Apr 24, 2014

... so application authors can determine whether their applications treats URIs with a trailing slash as a different resource than URIs without a trailing slash. Defaults to true. This option causes Router to set the 'routeTrailingSlashPattern' variable, which is added to the overall URIs matching regex. #848

…ter so application authors can determine whether their applications treats URIs with a trailing slash as a different resource than URIs without a trailing slash. Defaults to true. This option causes Router to set the 'routeTrailingSlashPattern' variable, which is added to the overall URIs matching regex. jashkenas#848
@braddunbar
Copy link
Collaborator

Hi @aholmes! Thanks for the patch.

Since the optional pattern was added I've been using the following, which I've found more flexible than a global option.

var Router = Backbone.Router.extend({
  routes: {
    // Slash is optional.
    'foo(/)': function() {
    },
    // Slash is required.
    'bar/': function() {
    }
  }
});

Did you already know about this? I saw your appeal for an option in #848, but I didn't see any mention of the technique above. For my money, the above approach is more flexible and explicit than a global option and requires a very small change.

@dgbeck
Copy link

dgbeck commented Apr 24, 2014

Damn, das why he famus

@aholmes
Copy link
Author

aholmes commented Apr 24, 2014

Hi @braddunbar,
Yes, I knew about the optional pattern. I think it was even mentioned in #848. For me personally, I don't like that solution, and a global rule on the structure of a URI fits my mental model of routing more nicely. It also means I don't need to worry about whether a route is missing the optional parameter when I want all my routes to behave the same. Thanks, though!

@aholmes aholmes closed this Apr 24, 2014
@aholmes aholmes reopened this Apr 24, 2014
@aholmes
Copy link
Author

aholmes commented Apr 24, 2014

Wrong button!

@braddunbar
Copy link
Collaborator

After mulling this over a bit, I think adding a global option is less flexible than the existing options. Adding (/) is not particularly difficult and doesn't lend itself to inconsistencies such as /foo.html vs /foo.html/.

If you absolutely must have it, you can always override Router#route.

var Router = Backbone.Router.extend({

  route: function(route, name, handler) {
    if (_.isString(route) && !/\(\/\)$/.test(route)) route += '(/)';
    return Backbone.Router.prototype.route.apply(this, arguments);
  }

});

@braddunbar braddunbar closed this Apr 29, 2014
@aholmes
Copy link
Author

aholmes commented Apr 29, 2014

If I can give one more shot at convincing you:

Using the configuration option allows me to treat a trailing / as significant or not across the board. I won't have to add (/) to any routes where I need this (which is all of them), and the configuration option is immediately clear to any one who isn't aware of (/).

Regarding flexibility, if someone is using the configuration option, they probably know why they are using it, and what the gains or losses are.

@akre54 akre54 mentioned this pull request Jun 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants