LinkView "active" class not always applied after retried transitions

[ehntoo]

I spotted an odd issue with the login workflow of a new app I started working on last week. After a user completed our login process, the navigation link for the route they came back to did not have an 'active' class applied.

I've reproduced the issue in a jsbin here: http://emberjs.jsbin.com/livisemixu/1/

To see the behavior, click on the "Secret" link, then the "Log in" link that appears in the route. There will be no active class when the transition is subsequently resumed. If you click back to the index and revisit the "secret" route, the class is then applied correctly.

Interestingly, if the model hook of the retried transition returns a promise that is not resolved immediately, the issue does not occur.

I've reproduced this both in 1.11.0-beta.4 and on canary, and will see if I can put together a failing test.

[topaxi]

Probably related to #10421

[engwan]

I encountered this same problem recently. And it is most likely related to #5208

I'm using ember-simple-auth and it does something similar. It's caused by the previousTransition.retry();

Based on the bug report, when you retry the transition, the router state isn't updated, so the link helpers are comparing their route to the wrong route.

More Info:

The router state is set in didBeginTransition but this is not called when a transition is retried.

[ehntoo]

Thanks, @engwan. The issue manifested in my app when using simple-auth as well, but I reduced it down to the testcase in the jsbin in the first post.

Based on my own investigation, I'd agree that #5208 is very likely the cause of this issue, or at least related.

[ehntoo]

I spent some more time looking into this without a whole lot of progress, but have implemented a (rather ugly) workaround by setting up the default model hook behavior for generated routes to return a promise.

  Ember.Route.reopen({
    model: function() {
      return new Ember.RSVP.Promise(function(resolve) {
        Ember.run.later(resolve, 1);
      });
    }
  });

You'll also need to be aware that all model hooks you override will also need to return a promise.

[pwfisher]

$100 and a Dollar Shave Club t-shirt to whomever fixes this bug. tweet to claim @pfisher42

[gerry3]

Just ran into this. Thanks @ehntoo for the workaround.

[barryofguilder]

I'm running into this still with Ember 2.4.0.

[acorncom]

The newly formed "issues team" is starting to run through the backlog on issues. It'd be really helpful to get a more recent reproduction of this issue (preferably in a Twiddle) so we can work on getting it fixed.

[ehntoo]

@acorncom - I've transcribed my jsbin from the original report into a twiddle here: https://ember-twiddle.com/5e6e66e8e1ecbea27e18d757d275899b . It appears to run without any deprecation warnings against release, beta, and even canary.

[acorncom]

That's quite helpful, yep I see it too ;-)

[Nav4e]

Is there some trick for hotfix?

[mattcoker]

@Nav4e My only workaround at the moment is to hardcode a transition to a specific route on login:

actions: {
  authenticate: function() {
    let credentials = this.getProperties('identification', 'password');
    this.get('session').authenticate('authenticator:custom', credentials).then(() => {
      this.set('errorMessage', null);
      this.transitionToRoute('targetRoute');
    }).catch(() => {
      this.set('errorMessage', 'Incorrect email or password. Try again.');
    });
  }
}

By calling transitionToRoute, it updates the router state.

[dwickern]

I worked around this by replacing transition.retry() with:

function * params(transition) {
  for (const info of transition.handlerInfos) {
    for (const value of Object.values(info.params)) {
      yield value;
    }
  }
}

const { url, name, queryParams } = transition.intent;
if (url) {
  this.transitionTo(url);
} else {
  this.transitionTo(name, ...params(transition), { queryParams });
}

[kjhangiani]

@dwickern you are a lifesaver, that issue has been plaguing me for a long time. That will do until there's a real fix for this issue.

Thanks!

[kjhangiani]

@dwickern actually ran into a bug with your implementation - because you used Object.values on params, (which is a keyed object), the params were coming out in an undefined order, resulting in the transition failing due to incorrect positional params

I changed it to:

function * contexts(transition) {
	for (const ctx of transition.intent.contexts) {
		yield ctx;
	}
}
const { url, name, queryParams } = transition.intent;
if (url) {
	this.transitionTo(url);
}
else {
	this.transitionTo(name, ...contexts(transition), { queryParams });
}

which fixed the issue for me

[dwickern]

@kjhangiani Good catch. Maybe you have a route with more than one dynamic segment like
this.route('my-route', { path: ':first/:second' })

If it works then the code can probably be simplified to

let { url, name, contexts, queryParams } = transition.intent;
if (url) {
  this.transitionTo(url);
} else {
  this.transitionTo(name, ...contexts, { queryParams });
}

[kjhangiani]

@dwickern thanks, that is indeed a lot simpler. It was due to multiple dynamic segments in the same route, yes.

I have updated my code to use your solution and it seems to work as expected. Thanks!

[pixelhandler]

@Nav4e @acorncom @barryofguilder @dguettler @dwickern @ehntoo @engwan @gerry3 @hbrysiewicz @jerel @kjhangiani @mattcoker @mike-north @pwfisher @topaxi is this still an issue, perhaps we should close or create a new reproduction of this, what do you think?

[gerry3]

we still have the workaround #10557 (comment) with Ember 2.18.0. would like to remove it if possible. maybe someone can confirm the issue has been fixed or create a reproduction on https://ember-twiddle.com