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

router.replaceWith calls pushState on the history object with refreshModel QP

Open mehulkar opened this issue 6 years ago • 7 comments

https://github.com/mehulkar/ember-example-replace-push-state

I think this may have the same root cause as #15801, but it manifests differently.

Problem

If a queryParam is configured to refreshModel: true, calls to router.replaceWith end up calling pushState on history instead of replaceState.

I'm not totally sure how this would manifest as a "bug" in a browser for an end user, but in my case, I have a custom Location using a custom history object, and calling pushState when we really meant to replace causes it to get an extra entry history entry.

Debugging

It seems that Ember (or underlying router.js lib thinks that because of the queryParams refreshModel config, this replaceWith transition is invalid, and it cancels it and generates its own.

Because router.replaceWith is implemented like this:

replaceWith() {
  this.transitionTo(...arguments).method('replace');
}

the transitionTo() promise gets replaced, and all the async stuff that causes the URL to update happens on the new promise, which does not yet have the urlMethod set to replace.

One possible solution here is that instead of using transitionTo(), a Transition object should be constructed with urlMethod set as part of the constructor, so that it does the right thing throughout its lifecycle, but that seems like a really big change.

mehulkar avatar Sep 20 '19 06:09 mehulkar

#5566 may also be related

mehulkar avatar Sep 20 '19 06:09 mehulkar

As recommended in https://github.com/emberjs/ember.js/issues/5566#issuecomment-64989276, I'm able to solve the issue by using the private queryParamDidChange method instead

mehulkar avatar Sep 20 '19 06:09 mehulkar

Converting to a native class in the route seems to break this workaround, so I might be back at square 1.

mehulkar avatar Oct 05 '19 00:10 mehulkar

I noticed something similar in my app (it could the exact same issue but it's a little hard to tell)

I put together a Twiddle that shows the problem we are facing

https://ember-twiddle.com/39fe48997dd6e62f6381e89d3c6aa74c?openFiles=routes.first.index%5C.js%2C

Essentially, the flow of the logic is

  1. Click on a link to a route
  2. A route within that path does a replaceWith to set some default query params
  3. Further down the routing "stack", another replaceWith is performed to redirect to different route within the routing hierarchy of the parent that added the query param

I would expect that the browser's history receives a single pushState with the final URL, at the "child" route, with the query param.

However, what actually happens is that an intermediary URL is pushed into the browser with replaceState, meaning that the location that the browser actually "lands" has a back button pointing to that intermediary state.

alexlafroscia avatar Apr 06 '20 19:04 alexlafroscia

This is still happening as of Ember 3.20.4.

I'm not totally sure how this would manifest as a "bug" in a browser for an end user

For our users, it means that they get stuck in a loop where they are unable to use the Back button anymore; doing do only removes the query param from the URL, which then re-enters the logic that calls replaceWith again to add the query param back in again.

I'm going to take a stab at adding a failing test to the Ember codebase for this.

alexlafroscia avatar Aug 12 '20 19:08 alexlafroscia

Hi!

We had the same issue recently but not with query-params.

We are using the replaceWith methods to make a feature with sub-routes feels like it is only one history entry, hence playing with the replace to navigate.

  • As long as navigation is triggered from the UI, Ember works normally and well;
  • When navigating from the browser (eg. browser navigation, back and forward arrows, using history.back(), or URLs), however, replaceWith, instead of making a replaceState, makes a pushState. In the console, it looks really close to what the reporter of this thread showed.

Also, seems like it could be related to what's written in the documentation (page is preventing-and-retrying-transitions):

However, if the browser back button is used to navigate away from route:form, or if the user manually changes the URL, the new URL will be navigated to before the willTransition action is called. This will result in the browser displaying the new URL, even if willTransition calls transition.abort().

To prevent the pushState, we found a workaround to abort the browser-triggered transition, and create a new one right away from Ember... And then the replaceWith works as intended.

frykten avatar May 24 '21 09:05 frykten

Just ran into this issue on a big production app. Its definitely something that customers can encounter, exactly like @alexlafroscia described above over a year ago

mattmarcum avatar Aug 10 '21 17:08 mattmarcum