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

RouterService breaks Route#intermediateTransitionTo #17430

Closed
bastimeyer opened this Issue Jan 5, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@bastimeyer
Copy link

bastimeyer commented Jan 5, 2019

I've encountered a bug while upgrading to Ember 3.6.1 and using the RouterService.

In the RouterService's init hook, it sets up listeners on the EmberRouter for the routeWillUpdate and routeDidUpdate events. These are used as a pass-through for the same events on the RouterService itself. However, in Ember's debug build, they also freeze the transition's routeInfo objects (from and to properties). See here:
https://github.com/emberjs/ember.js/blob/v3.6.1/packages/%40ember/-internals/routing/lib/services/router.ts#L272-L285
https://github.com/emberjs/ember.js/blob/v3.6.1/packages/%40ember/-internals/routing/lib/services/router.ts#L13-L21

This can cause issues in certain scenarios where the transition parameter is undefined and accessing from or to on the undefined transition will throw errors.

In my case, the transition parameter was undefined after calling transition.abort() in the app's SomeInitialRoute#beforeModel hook and then calling intermediateTransitionTo. Consider this example:

// this needs to be one of the app's initial routes, so for example the IndexRoute
export default IndexRoute extends Route {
  @service router;

  async beforeModel( transition ) {
    // access the RouterService, so it gets initialized
    this.get( "router" );

    // don't let the user access this route for now
    transition.abort();

    // show the loading screen instead
    this.intermediateTransitionTo( "loading" ); // <-- throws errors ⚠️⚠️⚠️

    // do some important stuff
    await new Promise( resolve => setTimeout( resolve, 1000 ) );
    // and eventually continue, eg. retry transition or go to a different route
    this.router.transitionTo( "foo" );
  }
}

I'm not entirely sure if this undefined transition parameter in the triggered events is intended. If it is, then the freezeRouteInfo method needs to be fixed. If it's not, then it must be an issue with RouterJS and its activeTransition property, which will stay undefined if the initial transition gets aborted, but I don't know anything about its implementation. However, this is where the undefined transition is coming from:
https://github.com/tildeio/router.js/blob/v6.2.0/lib/router/router.ts#L231-L238

@chadhietala

This comment has been minimized.

Copy link
Member

chadhietala commented Jan 8, 2019

Is it possible for you to create a reproduction of this?

@bastimeyer

This comment has been minimized.

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