emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.45k stars 4.21k forks source link

transition.abort doesn't stop the URL from updating #18620

Open apoorvparijat opened 4 years ago

apoorvparijat commented 4 years ago

transition.abort() is not currently working as expected. In some circumstances, when the user hits the back button, we prompt the user for unsaved changes. We handle this scenario in a lot of places by doing something like the following (This is the suggested approach in the tutorial here)

if (!confirm("You have unsaved changes")) {
  transition.abort();
}

The above doesn't appear to work 100% of the time. While the route has not changed, the browser URL points to something else.

Code for the above example I created a test ember app to replicate this. The code in this repo follows "Create your first ember app" tutorial guidelines. https://github.com/apoorvparijat/test-ember-app

Steps to reproduce

  1. Pull the code from the repo and start ember server
  2. Point your browser to http://localhost:4200/about route
  3. Click on Contact link
  4. You will see a confirmation dialog. The URL does NOT change at this point. Press OK here.
  5. You have now landed on Contact page - http://localhost:4200/contact
  6. Hit the browser back button now. You will see the confirmation dialog again. What I expect to happen: The URL stays the same on /contact What does happen: The URL has already changed to the /about page before accepting the dialog
  7. If you press Cancel, the URL and Ember app ends up in an inconsistent state. The URL points to /about but Contact route is loaded.

GIF demonstrating the same

Screen Recording 2019-12-18 at 04 15 pm

rwjblue commented 4 years ago

Thank you for reporting with so many details! I do think we should fix this (having the URL be mismatched with what is currently rendered is definitely not good), but it isn't obvious to me what the best way to fix would be.

Ember uses the History API's pushState method whenever there is an application triggered URL change (e.g. clicking the "Contact Us" link in your demo).

https://github.com/emberjs/ember.js/blob/04e324b7930247024915bcb32fbc75e91e3501ae/packages/%40ember/-internals/routing/lib/location/history_location.ts#L145-L152

Ember also listens for the popstate event to know when the user clicked the back/forward buttons to create in application transitions to navigate to corresponding URL. It is important to realize that at the point the event is fired, the history state has already been popped and the current URL in the browser has already been updated.

https://github.com/emberjs/ember.js/blob/04e324b7930247024915bcb32fbc75e91e3501ae/packages/%40ember/-internals/routing/lib/location/history_location.ts#L214-L225

The issue here is that the callback we register with onUpdateURL there initiates a transition, but does nothing to ensure that transition is successful. This means that if that transition is .abort()ed the UI will now be out of sync with the URL.

I think the best way to fix this, is to ensure that the URL matches after the transition completes.

rreckonerr commented 1 year ago

Hey! I also encountered this bug and I wonder how people deal with the issue