Added AutoLocation 'auto' option for automatic location setting based off support

[jayphelps]

Some people would like to support the history API but auto fallback to hashchanges in older browsers. The current way they could do that is check for support themselves and dynamically set the location property when they reopen the Router, but this makes it easy for them to not need that boilerplate code.

With the AutoLocation class, users can now just use:

App.Router.reopen({
  location: 'auto'
});

[gpoitch]

👍

[tomdale]

Any reason not to make this the default?

[jayphelps]

@tomdale When I first started using Ember, I actually (wrongfully) assumed this would be the default behavior, so I personally agree. Didn't make it the default since that's a potentionally breaking change so figured the core team would need to be in on that.

[jayphelps]

@tomdale By breaking I mean that anyone upgrading ember that was just using the default (hash) location would need to have a server rewrite in place otherwise page refreshes on any of the nested resources would 404.

[stefanpenner]

fantastic, but yes I agree it should not be default.

[jayphelps]

@stefanpenner Anything needed on this one further?

[stefanpenner]

One concern is what happens when someone using hash location, shares a link with someone who gets a different location implementation. This seems like it would cause issues.

[lukemelia]

I think the use case here is you want to support PushState, but you want a fallback for down-level browsers.

If that's true, then apps using this implementation would need a way to support converting a hash URL to the push state version, and vice versa. The latter would need to have a server component as well.

[trek]

👍 auto, 👎 as default for all the reason's above.

[wagenet]

This is a great idea but I think we have to address the case brought up by @stefanpenner where someone shares a history link with someone who only supports hash and vice-versa.

[jayphelps]

@wagenet @stefanpenner @lukemelia Agreed, a must have. I'll add it. We're already supporting this at SignNow (in house framework) so shouldn't take me long.

[stefanpenner]

I think if the caveats are solved to the extent they can be and well documented, this might be fine as an option.

[ebryn]

What's the latest thinking here?

[jayphelps]

@ebryn I'm working on adding support for accepting URLs in any structure even if that browser doesn't support that mode.

e.g. you set your app to use 'auto' location.

user A uses Chrome (which supports HistoryLocation/pushStates) to view this ember app and shares the current URL to user B: http://example.com/foobar

user B views the above url in IE9 (which does NOT support HistoryLocation, only HashLocation). Ember detects this and transforms the url to a hashed version and uses location.replace() to replace the history state with the hashed version, kicking off things as normal: http://example.com/#/foobar

This would work the other way as well. All this _is_ pretty stupid to have to support, but necessary for many people, myself included.

Regarding embercasts/embercasts#5; I think in that situation where a dev has changed the default 'hash' location to 'history' we could still support the URL and render the page but then don't hook any push, instead actually just navigate with location.href?

[stefanpenner]

@jayphelps awesome. Keep us in the loop!

[jayphelps]

@stefanpenner Sorry for being MIA. Vacation the last week. I've updated the repo/PR with the discussed changes. (I also brought my repo up to date with the upstream master so I could test against the latest)

Questions or issues let me know. Should be able to knock out any further changes rather quickly now that I'm back.

[jayphelps]

@stefanpenner Is this good to go for 1.1?

[stefanpenner]

Mr Api/consistency @tomdale thoughts?

@jayphelps I have a good feeling about this, I will have to play with it myself. One big thing though, is if we where to accept it we would need some pretty solid test coverage.

If in practice this is solid, it may make it into 1.0 final

[jayphelps]

@stefanpenner Are there any existing tests for the other Location classes I can use for reference? I'm not seeing any.

[stefanpenner]

@jayphelps maybe not :(

[jayphelps]

@stefanpenner Looking for clarification--would you still like for me to write tests for AutoLocation, even though the others do not have them? This weekend works well for me to wrap this up, if we can.

Also, @tomdale friendly bump

[stefanpenner]

@jayphelps that would be lovely.

[jayphelps]

@stefanpenner I'm having an issue writing a test. Maybe I'm missing something obvious (doh!) but when I create an Application, no Router is created inside my test case. When I added some debug statements to the Application class init (where the router is created) they aren't fired, leading me to believe Application is being overridden or something? Maybe intentionally for testing? I couldn't find any documentation or examples that helped.

var App;

module('Ember.AutoLocation', {
  teardown: function () {
    if (App) {
      Ember.run(function () {
        App.destroy();
      });
    }
  }
});

test('test 123', function () {
  Ember.$("#qunit-fixture").empty();

  Ember.run(function () {
    App = Ember.Application.create({
      rootElement: '#qunit-fixture'
    });

    // Router wasn't created/set
    App.Router === undefined;
  });
});

[machty]

@jayphelps try putting your test in https://github.com/emberjs/ember.js/blob/master/packages/ember/tests/routing/basic_test.js

It isn't ideal that everyone's putting their router tests here, but this kind of re-org belongs in its own PR, and shouldn't hold you up from merging in your change.

[jayphelps]

@machty Bummer but yep, will do.

[gpoitch]

Already using this in the wild and it's working well, nice job.
My only modification is for the history detect. Android support is buggy, so falling back to hash there.

Bug report:
https://code.google.com/p/android/issues/detail?id=23979

Modernizr's detect:
https://github.com/Modernizr/Modernizr/blob/master/feature-detects/history.js

[jayphelps]

@gdub22 👍 Right on!

[jednano]

@jayphelps Great presentation at the Ember Meet-up at HauteLook. I'm very excited to see this PR merged in. 💯

[oldfartdeveloper]

@jayphelps Agreed w/ @jedhunsaker 👍 get it in as soon as you can.

[nerdyworm]

I would like to see this merged. However it should not be the default due to the fact it requires the server side to play along.

[nixme]

How is this supposed to work in practice? I tried it out and when set means the router consumes the prefix when using HistoryLocation, and thus I had to include it when defining routes. But it doesn't know about it when using HashLocation. How are you reconciling that?

[jayphelps]

Basically, I'm not--currently. It definitely doesn't handle it correctly, just as you described. I'll likely make it a property on the App.Router and then have all location classes honor it. To expedite this initial PR, I may remove this broken functionality for now, then work on getting it working for a separate PR.

These catches are exactly what we need right now so thank you very much for reporting! Keep'um coming.

[jayphelps]

Looks like Router#rootURL already exists and glancing at the source looks to be passed along for the ride to the Location class, only used by HistoryLocation. Maybe I'm seeing it wrong, but this may be exactly the property I should be using. Digging deeper..

Edit: this is indeed the ticket.

[machty]

@jayphelps can you rebase against master and squash? I'd love to get this in shortly.

[jayphelps]

@machty @stefanpenner

Sorry everyone for the delay. I've been trying to resolve several issues I discovered while building robust tests. (who would have thought? har har) One of the more complex issues is that AutoLocation sometimes uses location.replace to replace the URL with the matching supported Location class, but obviously Ember continues to boot afterwards and can present the wrong route and even when not wrong, there is a noticeable rendering flash since the app finishes routing anyway.

I need a non-hacky way of preventing the app from finishing booting, or more technically, stopping it from continuing to route. If anyone has a suggestion, let me know. It would be great not to have to build "cancel router setup" functionality into the Router itself but I have not yet found a non-hacky way otherwise.

Can't use Application#deferReadiness because Router creation is not currently considered part of the application boot process. didBecomeReady has already fired before the Router is even created. (Router seems to be created implicitly in Application#startRouting via container lookup of router:main)

[gpoitch]

@jayphelps
I made a few changes/comments. This fixes the app rebooting/flashing when you start on a hash url on a history supporting browser.
Basically, replacing the above 'if' block:

if (get(this, 'supportsHistory')) {
  historyPath = get(this, 'historyPath');
  // should be safely assumed at this point?
  implementationClass = Ember.HistoryLocation;

  // use replaceState, not location.replace (causing full refresh)
  if (currentPath !== historyPath) {
    history.replaceState({}, null, historyPath);
  }
}

[jayphelps]

What about history => hash flashing? I think we need a clean, across the board way of canceling the routing process so neither flash.

[gpoitch]

I'm not sure that is going to be possible. You still have to replace the url, and there is no way to remove the pathname on hash only browsers without a full refresh.

[jayphelps]

Sorry, I should have clarified. I'm looking to stop Ember.Router from setting up the current route contexts and rendering the page so it remains blank during the refresh. I've already accomplished this pretty easily, but it feels hacky so was trying to get some input.

I basically passed a method similar to preventDefault() to the Location class creation.

function setupLocation(router) {
  var location = get(router, 'location'),
      rootURL = get(router, 'rootURL'),
      options = {};

  if (typeof rootURL === 'string') {
    options.rootURL = rootURL;
  }

  options.cancelRouting = function () {
    set(router, 'cancelRouting', true);
  };

  if ('string' === typeof location) {
    options.implementation = location;
    location = set(router, 'location', Ember.Location.create(options));

  }
}

Then when Router#startRouting is called, the router checks if cancelRouting is true and will ignore the routing request if so.

[gpoitch]

Yup, makes sense to cancel the rendering - I can't provide a better solution on that front.
As a separate addition then, the replaceState over location.replace does provide a better experience, when available.

[jayphelps]

Won't there still be a flash (even though it's quicker) because the wrong route will be rendered first?

e.g. In Chrome you visit /app/#/about, the Router will first render and display only IndexView first because the HistoryLocation class will be provided and not recognize the /about portion in the hash.

I can't test this until late tonight.

[gpoitch]

No, with my patch it is transitioning directly into /about.
As it stands, it starts up into index with NoneLocation, refreshes, and then starts in /about with HistoryLocation

[jayphelps]

Rad. Thanks for your help.

[wagenet]

@jayphelps What's the current status on this?

Also, can you squash some of these commits and follow the naming and flagging conventions listed here: http://emberjs.com/guides/contributing/adding-new-features/?

[jayphelps]

@wagenet Wanted to wait until the 1.0 release was out, now that it is I'll revisit and get it up to par with the new feature flags and all. Thanks for the reminder!

[wagenet]

@jayphelps Great!

[chrisvariety]

Does anyone know what happened to this commit? Seems github barfed a little. Is it in Ember 1.1.0 or 1.2.0-beta,1 ?

Thanks--

[stefanpenner]

this commit wasn't merged, it appears it only appears so because of a quirk within GH.

[chrisvariety]

Thanks ! Looks like it is here for anyone looking to try it out: https://github.com/jayphelps/ember.js/blob/auto-location/packages/ember-routing/lib/location/auto_location.js -- though it doesn't look like it is close to working with Ember 1.0.0 / 1.1.0