ember.js icon indicating copy to clipboard operation
ember.js copied to clipboard

Infinite loop when aborting query param transition opting in for refreshModel

Open matchwood opened this issue 9 years ago • 18 comments

The title says it all really. Aborting the transition sends the router into an infinite loop, see here:

http://discuss.emberjs.com/t/transition-abort-with-query-params-does-not-abort-transition/8830

JsBin here: http://emberjs.jsbin.com/luzaca/edit?html,js,console,output When you hit 'Go to 2' you should get a call stack exceeded error.

The same thing happens in 2.1. Which is not surprising because the bad code is in router.js - https://github.com/tildeio/router.js/blob/master/lib/router/router.js#L33 .

matchwood avatar Oct 11 '15 15:10 matchwood

@francisperp there is a workaround, by using another route hook to abort can you avoid the infinite loop created by the combination of QueryParam change and willTransition use?

  • http://guides.emberjs.com/v2.1.0/routing/preventing-and-retrying-transitions/

See this alternative implementation using the beforeModel hook: http://emberjs.jsbin.com/xobohu/6/edit?js,console,output

@locks This may be a documentation concern in the case where query param + refresh + willTransition + transition.abort are used an infinite loop is created. This should be avoided by using another hook to abort the transition, e.g. beforeModel.

pixelhandler avatar Oct 16 '15 19:10 pixelhandler

@pixelhandler Thanks for the guidance - good to know that it does work there. However the use case I have for this is handling transitions in the willTransition hook of my application route - willTransition bubbles up active routes and I can interact with it there. beforeModel does not bubble, There are many reasons for doing this - implementing any kind of global transition block for example.

I have actually hacked together a solution for my use case, which involved overwriting part of the router prototype. It took me 8 hours, and after spending that much time with the router code it seems to need a deep refactoring right the way through - the cause of the bug is that the code itself is overly stateful and circular, to the point where functions return values that may or may not have been registered to this.activeTransition after a recursive call. And all abort does is set this.activeTransition to null...

matchwood avatar Oct 16 '15 20:10 matchwood

Just wondering if this will be addressed as a bug? I don't think I have permission to label it as one!

matchwood avatar Oct 29 '15 22:10 matchwood

I am also running into this issue, I think it should be marked a bug.

I've played around trying to find a workaround but the best I've come up with is throwing an error within queryParamsDidChange. That does abort the transition, but it doesn't seem like the right thing to do.

bardhi avatar Dec 08 '15 23:12 bardhi

This is also affecting our app. The use case is we have a parent route with a list of items and some filtering QP's, and a child route that is an edit view. The childroute uses willTransition to abort the transition if there are unsaved changes.

In our case, we may just refactor away from this design to work around this issue.

thec0keman avatar Mar 22 '16 19:03 thec0keman

This is also causing issues in our app. This should be classified as a bug.

MichaelBoselowitz avatar Mar 24 '16 13:03 MichaelBoselowitz

I just stumbled over the same issue when trying to abort a transition using the following query parameter setup:

queryParams:{
    param: {
        refreshModel: true,
        as: 'alias'
    }
}

Hower, if all query parameters are at their default states, the call to transition.abort() works as expected. Is there any resolution planned in the near future?

arm1n avatar Aug 02 '16 13:08 arm1n

Just bumped into this issue. Are there any new suggestions on how to handle it?

boyanyordanov avatar May 16 '17 11:05 boyanyordanov

Got hit by this today also, my use case: need to abort transition + show a confirmation popup on willTransition if the model is dirty. Any ideas on how to handle this?

urbany avatar Jul 21 '17 11:07 urbany

I ended up saving a flag on the controller for the target route and then use that to deside whether or not to block the transition in the beforeModel hook of the target route.

The downside is that all of those controllers need to be defined explicitly because otherwise the resolver starts throwing errors.

boyanyordanov avatar Jul 21 '17 11:07 boyanyordanov

Ironically the app we work on ran into this bug as well. So we're borrowing from the idea here to work around: https://github.com/emberjs/ember.js/issues/9818#issuecomment-394623779

pixelhandler avatar Sep 11 '18 19:09 pixelhandler

@chadhietala - Any chance the router.js / router service updates for 3.6 have made this a bit easier to dig into (or possibly even fixed)?

rwjblue avatar Dec 10 '18 00:12 rwjblue

This is an issue in our app as well. I'd consider this a major bug with the ember router and am surprised it has not been addressed after more than 3 years.

r00b avatar Dec 14 '18 13:12 r00b

@francisperp there is a workaround, by using another route hook to abort can you avoid the infinite loop created by the combination of QueryParam change and willTransition use?

  • http://guides.emberjs.com/v2.1.0/routing/preventing-and-retrying-transitions/

See this alternative implementation using the beforeModel hook: http://emberjs.jsbin.com/xobohu/6/edit?js,console,output

@locks This may be a documentation concern in the case where query param + refresh + willTransition + transition.abort are used an infinite loop is created. This should be avoided by using another hook to abort the transition, e.g. beforeModel.

I was able to successfully work around this bug by calling transition.abort() in the beforeModel hook.

r00b avatar Dec 27 '18 15:12 r00b

Hmm, I just updated to 3.6 (well, 3.7 I guess), and I'm getting some sort of queryParam-related infinite loop where there wasn't one before. Not sure offhand if it's the same issue or just a shared symptom just yet, but something's up.

[EDIT] My own issue turned out to be https://github.com/ember-engines/ember-engines/issues/614 -- probably a red herring after all.

XaserAcheron avatar Jan 28 '19 22:01 XaserAcheron

I have the same issue in 2020 with 3.12(LTS).

mehran-naghizadeh avatar Apr 14 '20 07:04 mehran-naghizadeh

This is what happens for me when routing from a translation route to another:

In the app I'm working on, all translation routes extend one parent route. Therefore, they all share the same controller and since controllers are singletons, they share the same locale property. So, transitioning from a route with locale 'ES' to another route with locale 'FR', for instance, causes a second transition request by changing the locale queryParam. This second transition gets stuck in the abort<-->retry loop.

This is what I've figured out so far, I might be mistaken though.

mehran-naghizadeh avatar Apr 14 '20 13:04 mehran-naghizadeh

Still happening in 4.3.

My workaround—admittedly not great—is to toggle the refreshModel property to false before aborting the transition, and then back to true.

  async willTransition(transition: Transition) {
    if (this.controller?.changeset?.isDirty)
      // workaround for https://github.com/emberjs/ember.js/issues/12473
      this.queryParams.parentId.refreshModel = false;
      await transition.abort();
      this.queryParams.parentId.refreshModel = true;
    }
  }

oliverlangan avatar May 02 '22 19:05 oliverlangan

Still happening in 4.4 also.

Toggling the refreshModel property indeed seems to stop the infinite loop. However, it seems like the query parameters for the current route are being removed when aborting the transition.

lukasnys avatar Apr 14 '23 13:04 lukasnys