Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transition.isActive is undefined in Ember 2.8 #14268

Closed
blimmer opened this issue Sep 12, 2016 · 4 comments
Closed

transition.isActive is undefined in Ember 2.8 #14268

blimmer opened this issue Sep 12, 2016 · 4 comments

Comments

@blimmer
Copy link

blimmer commented Sep 12, 2016

Testing my LTS app against the current pre-LTS Ember 2.8 version, I noticed that some of my routes started breaking because I was checking transition.isActive in route hooks.

For instance, consider this block in a mixin to block mobile devices from accessing certain routes:

  beforeModel(transition) {
    this._super(...arguments);

    if (!transition.isActive) { return; }

    if (this.get('browserDetectService.isMobileDevice')) {
      this.transitionTo('mobile-not-supported');
    }
  }

In Ember 2.4.x LTS transition.isActive is defined to be a boolean. In Ember 2.8.0, transition.isActive is undefined.

To be honest, I can't remember why we were originally checking this, as it is an app that has been upgraded from early 1.x days. I also realize that this is likely caused by a router.js change, not something directly handled by Ember.

I think it will be safe for us to remove our check for that flag, but I wanted to call it out to make sure it's not an unintended change.

@rwjblue
Copy link
Member

rwjblue commented Sep 12, 2016

Seems like a regression to me.

@rwjblue
Copy link
Member

rwjblue commented Sep 12, 2016

Looks like tildeio/router.js@1db19a4 added this.isActive = undefined in the constructor to ensure the Transition object was properly shaped, but unfortunately that overrides the Transition.prototype.isActive = true; that is done here.

The fix is to remove isActive from the prototype and set the default value to true (instead of undefined) in the constructor. We should also add a test confirming it is true by default.

@rwjblue
Copy link
Member

rwjblue commented Sep 12, 2016

Should be fixed by tildeio/router.js#192 once it lands and is updated here.

@blimmer
Copy link
Author

blimmer commented Sep 13, 2016

Thanks @rwjblue - I really appreciate your quick turnaround on this! 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants