chaplinjs / chaplin

HTML5 application architecture using Backbone.js
http://chaplinjs.org
Other
2.85k stars 232 forks source link

Canonical URI handling with trailing slashes #532

Closed mehcode closed 10 years ago

mehcode commented 11 years ago

We should do something like:

# routes.coffee
match 'foo/bar', 'controller#action', 
  trailing: true  # URI with trailing slash is canonical (default)
  trailing: false  # URI without trailing slash is canonical
  trailing: null  # Perform no canonical redirecting

So if a user navigates to /foo/bar and trailing: true is set then the user will be redirected to /foo/bar/.

lukateake commented 11 years ago

Is there a real-world example of when an application would want each to go different places, i.e. with and without a trailing slash results in two distinct app locations?

mehcode commented 11 years ago

http://googlewebmastercentral.blogspot.com/2010/04/to-slash-or-not-to-slash.html


There isn't much benefit for a web application (there would still still exist two URIs that both point to the same place) however for a user example.io/user and example.io/user/ going to different places would confuse the hell out of them (for the average use case). I realize a file / folder browsing application would have a real use case for having example.io/user and example.io/user/ pointing to different places and that is fine. But for the common occurrence in web applications is that either both URIs exist and point to the same place or only one exists (ie. the developer forget to make two routes). I think chaplin should provide a way to make these redirects. Also for a web site running through something like node-chaplin (I know this doesn't exist yet) that would fork for bots and run chaplin on the server producing the HTML for bots to consume, this would give only one URI for SEO concerns.

lukateake commented 11 years ago

My immediate impression: this is a configuration detail at the application level. Drawing solely from my own experience, the notion of redirecting URIs to trailing slashes is either always on or always off. This is in contrast to individual configuration at the route level via match per @mehcode's example above.

It might be easier to maintain code by passing a variable in the options hash that's ultimately sent into the Router's constructor method in order to override a sensible default for all routes. (I prefer that Chaplin automatically redirects to a trailing slash if one is not provided.)

chrisabrams commented 11 years ago

+1 to configuring it at the application level.

I prefer that Chaplin automatically redirects to a trailing slash if one is not provided.

I'm the opposite, but that's why configuration is great :)

mehcode commented 11 years ago

redirecting URIs to trailing slashes is either always on or always off

Great point.

# app.coffee
initialize: ->
  # the trailing slash that would apply to all routes.
  @initRouter routes, trailing: true  
lukateake commented 11 years ago

Actually, now that I visualize the sharing of hyperlinks and other use cases, I'm of @chrisabrams' mindset, too!

No trailing slashes ever! (Not enough coffee this morning and I'm not mentally back from vacation.)

paulmillr commented 11 years ago

trailing: false or trailing: true should be default? I think false; github.com/paulmillr is more cute than github.com/paulmillr/

mehcode commented 11 years ago

Sure. trailing: false can be a default option. Looks like its got the most votes.

# chaplin/application.coffee
initRouter: (routes, options) ->
  @router = new Router routes, _.defaults options,
    trailing: false
paulmillr commented 11 years ago

similar issue: 404s. what is good approach to them in webapps?

mehcode commented 11 years ago

Currently I do (and I'm sure a lot do the same) something like:

# routes.coffee
# lots of routes [...]
match '*url', 'error#404'

I would appreciate a semantic method to define a 404 route. Additionally a semantic way to say "route to the 404 page" if a user routes to /document/51 and the document #51 doesn't exist. No idea on the syntax at the moment.

akre54 commented 11 years ago

@paulmillr: couple options:

  1. The return value for Backbone.history.start() is false if no route matches. initRouter bubbles this return value to application (example)
  2. Add a splat as last route in your router as a catch-all, and point to your error controller.

documentcloud/backbone#2455 just landed in master so idea 1 can also be applied to link routing from Dispatcher#dispatch.

molily commented 11 years ago

A catch-all as the last route are an anti-pattern in my opinion. All of our single-page apps have URLs that are not routed inside the JS app. When I click on the link and the Layout doesn’t find a matching route, the browser can perform a normal HTTP request that may result in a download or leaving the single-page app (probably into another JS app on the same domain).

With a catch-all route, I need to give all those “external” links a noscript class. In my opinion, it should be a server task to handle 404s – the server needs to do that anyway if you want proper URLs, search engine accessibility etc. Your app still needs to handle specific 404s on a controller level (for example, some article on Moviepilot might point to /movies/gone-with-the-wind-2 – there’s a route for /movies/:id but no such movie to my knowledge).

Additionally a semantic way to say "route to the 404 page" if a user routes to /document/51 and the document #51 doesn't exist. No idea on the syntax at the moment.

Yep, but that’s something you can’t decide in the router. The controller needs to fetch the model in order to decide whether it exists. A general error handler could cover responses like 401, 404/410, 500, 503…:

model.fetch().then(success, @handleFetchError)

I would define handleFetchError on the application’s Controller class. I don’t see how Chaplin can offer more than such conventions. A predefined error handler and error pages probably?

vincentwoo commented 11 years ago

Has there been any movement on this issue?

paulmillr commented 11 years ago

nope

paulmillr commented 10 years ago

DONE!