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

Infinite loop when aborting query param transition opting in for refreshModel #12473

Open
matchwood opened this issue Oct 11, 2015 · 19 comments
Open

Comments

@matchwood
Copy link

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 .

@pixelhandler
Copy link
Contributor

@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?

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.

@matchwood
Copy link
Author

@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
Copy link
Author

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

@bardhi
Copy link

bardhi commented Dec 8, 2015

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.

@thec0keman
Copy link

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.

@MichaelBoselowitz
Copy link

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

@arm1n
Copy link

arm1n commented Aug 2, 2016

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?

@boyanyordanov
Copy link
Contributor

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

@urbany
Copy link

urbany commented Jul 21, 2017

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?

@boyanyordanov
Copy link
Contributor

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.

@pixelhandler
Copy link
Contributor

pixelhandler commented Sep 11, 2018

Ironically the app we work on ran into this bug as well. So we're borrowing from the idea here to work around: #9818 (comment)

@rwjblue
Copy link
Member

rwjblue commented Dec 10, 2018

@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)?

@r00b
Copy link

r00b commented Dec 14, 2018

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
Copy link

r00b commented Dec 27, 2018

@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?

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.

@XaserAcheron
Copy link

XaserAcheron commented Jan 28, 2019

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 ember-engines/ember-engines#614 -- probably a red herring after all.

@mehran-naghizadeh
Copy link

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

@mehran-naghizadeh
Copy link

mehran-naghizadeh commented Apr 14, 2020

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.

@oliverlangan
Copy link

oliverlangan commented May 2, 2022

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;
    }
  }

@lukasnys
Copy link

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.

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