Support omitting trailing slashes in rootURL with AutoLocation

[eventualbuddha]

In v1.5.1 AutoLocation asserts that rootURL ends with a slash. This breaks any apps that have e.g. /dashboard as a rootURL. We should fix this for v1.6. I plan on submitting a PR soon.

[gpoitch]

Here is a small nodejs app where you can test serving with or without a trailing slash: https://github.com/gdub22/ember-autolocation-slash-tests

[jayphelps]

From @gdub22's example we found that both HistoryLocation and HashLocation assume a rootURL that ends with a trailing slash, even though neither assert so. This is demonstrated by navigating to /dashboard then clicking on the Index link-to. A new browser history state is created, even though you haven't left. This needs to be addressed as well unrelated to AutoLocation.

[gpoitch]

Here is a hosted example that is configured to remove trailing slashes server side:
http://limitless-coast-2138.herokuapp.com/dashboard

Note that the assert added in #4717 had to be removed in order to get the app to boot

[eventualbuddha]

Unfortunately I haven't done this and likely won't have time in the near future. If someone else wants to tackle it, be my guest.

[wagenet]

Closing due to inactivity. If someone wants to pick this up again, let me know.

[msgerbush]

I've also run into this issue. I might be able to take a stab at fixing it, if someone can point me in the right direction.

[wagenet]

@machty can you provide direction here?

[msgerbush]

you know what it doesn't look too bad, i'll just take a stab at a PR and you guys can correct me if I'm wrong.

[alexgorbatchev]

Hey friends, I'm picking up where @eventualbuddha left off and am wondering what is the rationale behind this breaking change? We are trying to upgrade from 1.5 to 1.8 and this is breaking our routes.

[jayphelps]

@alexgorbatchev See the conversation in #4717

tl;dr we need the consistent convention to know whether or not the path needs transforming AutoLocation. /foo/bar/#/about <=> /foo/bar/about. Now consider the index route: /foo/bar/ is the same path for both hash and history because the ending slash means index; we're basically enforcing that convention. Make sense? It would be great to know why you went with to omitting the trailing slash though so please lmk.

[jayphelps]

@alexgorbatchev Also note my previous important comment about extraneous history states for index and more discussion at #4793

It may seem like a simple problem at first glance but from what I can tell and my experience w/ the location classes, it's non-trivial to support both but very trivial to change your server's configuration to use the trailing slash. (and 301 old route if needed)

[alexgorbatchev]

@jayphelps It's an existing ember/rails app that is currently on ember 1.5. I'm currently testing the following monkey patch to work around this without having to modify ember.js

_create = Ember.AutoLocation.create
Ember.AutoLocation.create = ->
  _assert = Ember.assert

  Ember.assert = (msg) ->
    Ember.assert = _assert
    return if msg is 'rootURL must end with a trailing forward slash e.g. "/app/"'
    _assert.apply @, arguments

  _create.apply @, arguments

[jayphelps]

@alexgorbatchev Mind using plain JS for people who don't know CS?

[jayphelps]

@alexgorbatchev Disabling the assertion is okay, just keep in mind it's there for a reason. At least one known bug, possibly more since the location classes aren't written and tested to handle it.

[alexgorbatchev]

var _create;

_create = Ember.AutoLocation.create;

Ember.AutoLocation.create = function() {
  var _assert;
  _assert = Ember.assert;
  Ember.assert = function(msg) {
    Ember.assert = _assert;
    if (msg === 'rootURL must end with a trailing forward slash e.g. "/app/"') {
      return;
    }
    return _assert.apply(this, arguments);
  };
  return _create.apply(this, arguments);
};

[alexgorbatchev]

@jayphelps thanks for heads up! I'll be testing the whole app tomorrow to see what comes up.

[jayphelps]

@wagenet I believe this can be closed as something we can't realistically support. If someone has gobs of time on their hands, I'd love to be proved wrong. Every approach I've tried has leaked.

[gpoitch]

This still trolls me on new projects. @jayphelps is this even fixable before I dive in? How about adding an app option that can bypass the assert?

[jayphelps]

@gdub22 it has been too long for me to remember if it's fixable but you're in unsupported waters if you do bypass that assertion because the location classes absolutely do not knowingly support it, e.g. Extraneous history states at the very least

Why does it troll you? I'm legitimately asking...why would you intentionally want to use a different convention? The trailing slash has a real meaning. It means index.

[jayphelps]

@gdub22 Also see #10201 (comment)

[gpoitch]

@jayphelps I find myself writing server code every time to redirect /dashboard => /dashboard/ because users entering the former face a blank page and an exception. Going to look into it again and if not fix it, maybe come up with a better user/developer experience out-of-the-box for that situation. Appreciate your time spent investigating and fielding questions on this topic.

[jayphelps]

That seems like a fair thing to write..? Just like you might write a redirect to add www.

On Apr 24, 2015, at 3:29 PM, Garth Poitras notifications@github.com wrote:

@jayphelps I find myself writing server code every time to redirect /dashboard => /dashboard/ because users entering the former face a blank page and an exception. Going to look into it again and if not fix it, maybe come up with a better user/developer experience out-of-the-box for that situation. Appreciate your time spent investigating and fielding questions on this topic.

—
Reply to this email directly or view it on GitHub.