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

Route queryParams replace does not work if refreshModel is true #11843

Closed
novtor opened this issue Jul 21, 2015 · 13 comments
Closed

Route queryParams replace does not work if refreshModel is true #11843

novtor opened this issue Jul 21, 2015 · 13 comments

Comments

@novtor
Copy link

novtor commented Jul 21, 2015

For a route with queryParams, replace does not work if refreshModel is set to true:
http://jsbin.com/jebuko/edit?html,js,output
Open jsbin as live preview, enter something in input field, submit the input, it will modify query parameter 'a' and refresh the template. Do it several times. Then 'back' button will pass trough all the submit history.
Is it expected or I do something wrong?

@pixelhandler
Copy link
Contributor

@novtor can you take a look at #10945 to see if this is a duplicate issue?

@stefanpenner
Copy link
Member

will reopen if it is demonstrated to not be a duplicate

@novtor
Copy link
Author

novtor commented Jul 27, 2015

No it is not duplicate. I opened another one some time ago: #10972 which is a duplicate, but this one is independent.
In fact, #10945 and #10972 say that the transition retry does not work with refreshModel query params and dynamic segments : there is an exception thrown, stacktrace is present in console, there is a problem with the page visualisation.
The issue I describe here is that independently of dynamic segments, if you put in a route:

App.ArtistSongsRoute = Ember.Route.extend({
queryParams: [
  qp: {
     refreshModel: true,
     replace: true
  }
]
....

This should result in, AFAIU, that if the model is refreshed several times with different 'qp', the browser history will only hold one reference on this route url. So, let's say the user searches for an artist:
artist/someartist.
and then looks for artist songs:
artist/someartist/songs?start=1980&end=1981
and then
artist/someartist/songs?start=1981&end=1982
and then, let's say
artist/someartist/songs?style=rock
this would only create one entry in the browser history for artist's songs and if the user clicks 'back' browser button, she should be routed to the "artist/someartist" route.

http://emberjs.jsbin.com/zawiwe

enter '1' in the input and submit, then '2' and submit, then '3' and submit. Now if you click 'back' browser button you will work through &qp=2 and &qp=1 history.

On the other hand, if you use 'Send with replace query params', this works as expected, no history is pushed

@novtor
Copy link
Author

novtor commented Jul 29, 2015

@stefanpenner, is my explanation clear? Do you think it is not duplicate issue?

@stefanpenner stefanpenner reopened this Jul 29, 2015
@richmolj
Copy link

richmolj commented Dec 2, 2015

I'm experiencing this on Ember 2.2 as well. replace is ignored if refreshModel is true.

@novtor what do you mean by "send with replace query params"? Looking for a workaround.

@novtor
Copy link
Author

novtor commented Dec 2, 2015

hi @richmolj , see the jsbin example

@Panman82
Copy link
Contributor

Panman82 commented Dec 3, 2015

This is a duplicate of #10577 ??

@pixelhandler
Copy link
Contributor

@novtor can you provide an updated example (reproduction of the issue) using a current release of Ember.js or the LTS release (2.4.6) ?

@pixelhandler
Copy link
Contributor

pixelhandler commented Aug 19, 2016

@novtor I created a current example using ember-twiddle, see

I set the replace: true as you described and do notice that the back button in the browser still has an entry for each param I submit.

@pixelhandler
Copy link
Contributor

@emberjs/issues-team @emberjs/learning-team-managers according to the docs The example is to either use refreshModel: true or replace: true not both.

I'm curious if this is a documentation issue or a bug.

Assuming you could use:

  queryParams:{
    a: {
          refreshModel: true,
          replace: true
    }
  },
  model: function(params) {
    return ['red-'+params.a, 'yellow', 'blue'];
  }

… and not introduce new history to the browser, this would be a bug.

However in Ember the URL is intended to represent the state of the app and a refresh of the model (even in combination of the query param) is a different state of the application. Which would indicate that using both refreshModel: true or replace: true should be avoided.

@mike-north
Copy link
Contributor

Almost certainly same as #10577 as @Panman8201 points out.

As @pixelhandler points out, the guides never use these two configuration options together in the same example. When I teach queryParams I also use a separate example for each config option because of this limitation/bug.

It would be a big help to know which of the following situations this issue falls into

  1. The guide/docs describe intended feature set accurately and in completeness. replace and refreshModel are mutually exclusive options by design. This would be a surprise to me because from a consumption standpoint, the two concepts we're concerned with here seem only loosely related (read: an either/or tradeoff is neither expected nor beneficial)
    • push vs replace re: the browser history stack
    • which route lifecycle hooks will be called
  2. The guide describes a subset of the intended feature set, and the intent is to allow replace and refreshModel to be used together for the same queryParam. In this case, let's get this issue or LinkView "active" class not always applied after retried transitions #10557 tagged as a bug. I am happy to submit a failing test case.
  3. We're not sure. This requires more core team discussion. If this is the case, I'd like to lobby that it be discussed sooner rather than later. The current behavior gets in the way of some simple & common use cases that teams bump into while exploring their options for tech stack choices (i.e., typeahead-style server side filtering of a list, with search pattern serialized to the URL via data binding).

Happy to pull some weight on this once we know what we're looking at.

@lcoq
Copy link
Contributor

lcoq commented Feb 23, 2017

@pixelhandler @machty @mike-north @alexspeller is there any update on this issue ?

I wonder if tildeio/router.js#198 will fix this ?

@acorncom
Copy link
Contributor

This was fixed by #15148, thanks folks!

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

9 participants