rendrjs / rendr

Render your Backbone.js apps on the client and the server, using Node.js.
MIT License
4.09k stars 313 forks source link

Add a leading / to backbone route misses to prevent routes being appended #479

Closed bigethan closed 9 years ago

bigethan commented 9 years ago

in a situation where there was a link like

<a href="/path/not/in/rendr/routes">stuff</a>

On the page at www.example.com/user/123. The app would end up redirecting to:

www.example.com/user/123/path/not/in/rendr/routes

instead of

www.example.com/path/not/in/rendr/routes

due to the path that's passed to the client router never having a leading slash.

If a link is known to not be in Rendr, you can add the data-pass-thru element to the link and everything will work properly. We can't always do that since we have separate Rendr apps on /foo and /bar that link to each other with shared code that shouldn't always be data-pass-thru links. I also think that this PR matches the intent of the code.

I kinda think that this used to work, but may have changed in the Backbone update (there were some other breaking changes, too lazy to try and old version and see :-). But really, I don't know. We're doing some new stuff with our Rendr. The breaking HTML concern here is that it prevents relative urls that don't have routes. E.g. using the /user/123 setup above but linking to:

<a href="photos">user's photos</a>

but wanting /user/123/photos to miss Rendr.

I don't know how popular this particular code path is, so I'm very much willing to be told that it's working fine. Or that instead of using window.location.href it should just use Backbone.router.jeremysRedirect(path) or something. I don't know the Backbone router internals that well (I know them ok, but could certainly be missing a method / option combo feature).

The express side router handles route misses properly.

/cc @saponifi3d @alexindigo @purusho

alexindigo commented 9 years ago

:+1:

ingiulio commented 9 years ago

not sure what's happening with Travis, but :+1: for the PR

saponifi3d commented 9 years ago

Re-running travis, if it's green then i'll merge, this all looks good :dancers: