replaceWith broken on routing during page load

[jamesarosen]

Given an app with the following:

// router.js
this.route('foos', function() {
  this.route('show', { path: '/:fooId' });
});

// route:foos
model() {
  return [ 'one', 'two', 'three' ];
}

// route.foos.index
beforeModel() {
  const foos = this.modelFor('foos');
  this.replaceWith('foos.show', foos[0]);
}

1. open a new tab
2. type https://yahoo.com in the location bar and hit enter
3. type https://google.com in the location bar and hit enter
4. type http://myapp.local/foos/ in the location bar and hit enter
5. press the Back button

You are taken back to Yahoo, not Google.

If I were to change replaceWith to transitionTo, then the problem becomes a new one:

1. load the app
2. click on a link to /foos/
3. redirected to /foos/one (correctly)
4. press back
5. redirected to /foos/one (incorrectly)

[jamesarosen]

We tried the following, which mostly works:

Ember.Route.reopen({
  redirectTo(transition, route, params) {
    if (transition.sequence === 0) {
      if (params) { this.replaceWith(route, params);
      else { this.replaceWith(route); }
    } else {
      if (params) { this.transitionTo(route, params);
      else { this.transitionTo(route); }
    }
  }
});

Then I would change my route to

// route.foos.index
beforeModel(transition) {
  const foos = this.modelFor('foos');
  this.redirectTo(transition, 'foos.show', foos[0]);
}

This works unless we call redirectTo(x) which then calls redirectTo(y). In that case, the transition.sequence has been incremented and the y redirect is a transitionTo, which is wrong.

[jamesarosen]

replaceWith

1. Open a new tab
2. Go to https://google.com
3. Go to https://yahoo.com
4. Go to http://emberjs.jsbin.com/pumeje/7
5. The routes will move you to #/books/1
6. Press Back. You will go to Google instead of Yahoo

transitionTo

This only happens with HistoryLocation (and possibly only with <base> as well)

1. Open a new tab
2. Go to http://emberjs.jsbin.com/pumeje/14/
3. The routes will move you to http://emberjs.jsbin.com/pumeje/14/books/1
4. Press Back. You will stay on http://emberjs.jsbin.com/pumeje/14/books/1

[Turbo87]

FYI the same thing happens for redirect() instead of beforeModel()

[nathanhammond]

I'm thinking that we can write tests for this by inspecting window.history.length. My first goal is to actually get some reasonable tests, and then figure out how to solve them. 😄

[bendemboski]

I found what looks like a better workaround for this issue in case anybody is interested. The Transition objects have a urlMethod property that indicates how the URL is going to be updated when the transition completes. In particular, it's falsey when the URL is not going to be updated, e.g. because this is the initial transition on app boot.

So, substituting if (!transition.urlMethod) for if (transition.sequence === 0) should work. It relies on the property that the router actually checks when deciding what to do at the end of the transition, rather than on the sequence property that just coincidentally tells us, so is probably a better choice of undocumented private properties :)

Also, I've submitted a PR to document it and make it public, so if that goes through then this workaround will be on more solid ground.

[alexspeller]

Fixed by tildeio/router.js#197

[bgentry]

@alexspeller I think something might still be broken here. I'm attempting to do a replaceWith in the beforeModel to set a default query param (which I want to be visible in the URL):

export default Route.extend({
  beforeModel(transition) {
    if (!transition.state.queryParams.timeframe) {
      this.replaceWith({ queryParams: { timeframe: '24hrs' } });
    }
  }
}

If I make the same replaceWith call in an action on the route, it works as expected. I'm sure that my controller is set up correctly for queryParams because they work if I transition elsewhere:

export default Controller.extend({
  queryParams: ['timeframe'],
  timeframe: null,
});

This is on v2.10.0-beta.3 (also tried canary).

[rwjblue]

@bgentry - I suspect that the issue you are seeing is query param specific, and would likely justify a separate issue. Would you mind making one?

[bgentry]

@rwjblue opened #14606

[rwjblue]

Main issue fixed by tildeio/router.js#197