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

Cleanup eager URLs after abort #5210

Closed
ef4 opened this issue Jul 22, 2014 · 28 comments
Closed

Cleanup eager URLs after abort #5210

ef4 opened this issue Jul 22, 2014 · 28 comments

Comments

@ef4
Copy link
Contributor

ef4 commented Jul 22, 2014

Eager URL updates are nice when everything manages to abort or transition in the same runloop. But aborts & transitions can happen in subsequent run loops, and in that case we leave the application in a broken state.

In the abort case, you end up with a URL that doesn't reflect your real current state. The next time the user hits the back button, nothing happens.

In the redirect case, you break the back button by leaving an intermediate state in the user's history, which will often just redirect them back again.

I discussed this with @machty and we agreed it would be good to work on this. Most likely the router can keep track of eager URL pushes and unwind them with history.back() or similar when an abort happens.

@SirZach
Copy link

SirZach commented Jul 25, 2014

I agree with this issue. I'm seeing it now, even if I do an transition.abort() in the willTransition hook of the route. The URL reflects the page that it would have gone to if the transition wasn't aborted.

@stefanpenner
Copy link
Member

👍

@machty
Copy link
Contributor

machty commented Aug 3, 2014

@ef4 I'd love for you to take a stab at this if you have the inclination; lemme know your interest/availability

@ef4
Copy link
Contributor Author

ef4 commented Aug 3, 2014

I can work on this, but probably not right away.

@mutewinter
Copy link
Contributor

👍 ran in to this when following the willTransition sample code in the API docs. The URL is changed even though abort was called.

@bcardarella
Copy link
Contributor

I've got the same problem. I created a jsbin recreating it for the curious: http://emberjs.jsbin.com/tijebi/1

@wagenet
Copy link
Member

wagenet commented Nov 1, 2014

@ef4 still planning to look at this?

@ef4
Copy link
Contributor Author

ef4 commented Nov 6, 2014

Sorry, it's low on my priority list of late.

@cibernox
Copy link
Contributor

For the record, I've just experienced the same issue

@machty
Copy link
Contributor

machty commented Dec 15, 2014

We're getting rid of eager URLs, closing in favor if #9919

@kottenator
Copy link

Just in case if someone still have this problem and requires a solution right now:

// app/routes/your-route.js

export default Ember.Route.extend({
    // ...
    actions: {
        willTransition(transition) {
            var model = this.controller.get('model');
            if (
                model.get('hasDirtyAttributes') && 
                !confirm("You're going to discard all unsaved changes. Are you sure?")
            ) {
                transition.abort();

                // Custom revert of Back button result
                var oldURL = this.router.generate(this.routeName, model);
                var newURL = this.router.location.getURL();
                if (oldURL != newURL) {
                    this.router.location.setURL(oldURL);
                }
            } else {
                return true;
            }
        }
    }
});

I don't like to use this.router.location methods directly but it's like the only way to do this right now (to handle all types of locations)

@bcardarella
Copy link
Contributor

I just updated my jsbin from last year to the 1.13.4 and this issue still exists: http://emberjs.jsbin.com/lohekasuhu/edit?html,css,js

@stefanpenner stefanpenner reopened this Jul 17, 2015
@adamreisnz
Copy link

I can confirm that this is still an issue with 1.13.11 as well.

Calling abort() on a transition in afterModel results in the URL being updated and the application getting into a broken state.

Subsequent transitionToRoute calls failed to correctly update the route, and the above workaround no longer works as router.location doesn't have getURL and setURL methods.

@Serabe
Copy link
Member

Serabe commented Apr 17, 2016

I updated @bcardarella's [jsbin to 2.5.0]. Still happening.

@rwjblue
Copy link
Member

rwjblue commented Nov 11, 2016

This was fixed by tildeio/router.js#197 (in 2.10.0-beta.3+).

Demo against v2.10.0-beta.3: http://emberjs.jsbin.com/yeqisuh/1

@rwjblue rwjblue closed this as completed Nov 11, 2016
@kanderek
Copy link

kanderek commented May 6, 2017

@rwjblue I'm not sure this is fixed for the abort case yet. I'm seeing this issue in ember 2.12.0. When I transition to a route which is aborted in the model hook the url reflects the state of the application as if the transition was not aborted. Are others still experiencing this behavior of transition#abort?

@bschouwerwou
Copy link

@kanderek Yes, I just came across this behaviour. I'm on 2.11.3 though.

@woprandi
Copy link

Hi,
with ember 2.14, the issue still seems to be present

@dkorenblyum
Copy link

dkorenblyum commented Jul 17, 2017

Experiencing this issue with 2.11.3

Currently using this ugly workaround I found on stackoverflow

//right after an aborted transition
if (window.history) {
  window.history.forward();
}

Please dont shame me for that ^

@rafaelbnp
Copy link

+1

I'm still seeing this issue! I'm on 2.12.2

@wagenet wagenet reopened this Oct 19, 2017
@wagenet
Copy link
Member

wagenet commented Oct 19, 2017

Can someone provide a failing example for this?

@btecu
Copy link
Contributor

btecu commented Apr 26, 2018

@wagenet here is an example using Ember 3.1.1:
https://github.com/btecu/ember-issues/tree/5210

@pixelhandler
Copy link
Contributor

@SirZach @adamreisnz @bcardarella @bschouwerwou @btecu @cibernox @dkorenblyum @ef4 @kanderek @kanongil @kottenator @machty @mutewinter @pixelhandler @rafael-paiva @rwjblue @stefanpenner @wagenet @woprandi is this still an issue, perhaps we should close or create a new reproduction of this, what do you think?

@pixelhandler
Copy link
Contributor

@btecu could you update your example to Ember 3.5 ?

@linearza
Copy link

linearza commented Nov 9, 2018

+1 Running into this now as well (i think). In our case we transition into a page, then go back using window.history.go(-1) - it seems to just continue loading the current page (completing transition etc and loading rest of async content) which takes a while, then eventually makes the transition back. The url updates immediately though. Possibly because we use the history api directly?

@miguelcobain
Copy link
Contributor

Just experienced this with 3.9.1

@ef4
Copy link
Contributor Author

ef4 commented May 31, 2019

This issue is muddled because the original bug definitely did get fixed. We don't do eager URL updates anymore. I'm going to close because there's a lot of irrelevant history here.

@miguelcobain if you can please share your reproduction on 3.9.1 as a new issue, that will help it get proper attention.

@ef4 ef4 closed this as completed May 31, 2019
@Mitchal
Copy link

Mitchal commented May 5, 2020

This was the top result when I searched for this issue that still seems to be a thing, so I thought I'd paste an updated snippet with a workaround for anyone who stumbles across this (please let me know if there is an actual solution that I'm not aware of):

  @service router

  @action 
  willTransition(transition) {
    if (!transition.to.find(route => route.name === this.routeName) && !confirm('confirm?')) {
      transition.abort()
      let oldURL = this.router.currentURL;
      let newURL = this.router.location.getURL();
      if (oldURL != newURL) {
          this.router.location.setURL(oldURL);
      }
    }
    return true;
  }

This is a simplified version of the suggestion @kottenator gave above (#5210 (comment)).

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