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

[Bug] Query param sticky value lost #19307

Closed
patsy-issa opened this issue Dec 15, 2020 · 13 comments · Fixed by #19448
Closed

[Bug] Query param sticky value lost #19307

patsy-issa opened this issue Dec 15, 2020 · 13 comments · Fixed by #19448

Comments

@patsy-issa
Copy link
Contributor

🐞 Describe the Bug

Query parameters lose their sticky value and revert to the default defined within the controller of the parent route when transitioning to a child route. Both the parent and child route have slow model hooks and loading substates. The last working version that does not have this bug is ember-source 2.22.0, all versions since then up to 2.24.0-beta.2 contain this bug.

#19249 was aimed at resolving this issue but it seems the use case presented in the reproduction might have slipped by.

🔬 Minimal Reproduction

  • Checkout the reproduction repository
  • yarn install & yarn start
  • Navigate to /media
  • click on the movies link
  • select an item from the movie list

😕 Actual Behavior

During the transition the list will revert back to the albums for a short duration before transitioning into the correct route. This is due to the filter query parameter's value being set to its default value when the loading route is activating.

ember-qp-bug

🤔 Expected Behavior

The filter query parameter's value does not change when the loading route is active.

🌍 Environment

  • Ember: ^2.22.1
  • Node.js/npm: 14.1.0/6.14.4
  • OS: macOS Catalina 10.15.6
  • Browser: Brave Version 1.18.70

➕ Related issues

@rreckonerr
Copy link
Contributor

Yeah, router_js v7.1.1 is the cause for it. Specifically, tildeio/router.js#308 that aimed to deal with query params serialization errors and introduced this bug. Apparently, tests didn't take into account the intermediate state when query params were reset to default values. 😥

cc @rwjblue

@rreckonerr

This comment has been minimized.

@rreckonerr
Copy link
Contributor

rreckonerr commented Feb 13, 2021

Seems like this issue got fixed somewhere along the way. Tested it on 3.27.0 and here's the result:
UPDATE: yarn linking didn't work as expected, so the bug is still out there 🐞

@sly7-7
Copy link
Contributor

sly7-7 commented Mar 2, 2021

@rreckonerr I just hoped 3.25 would resolve my issue as well, but it doesn't 😢 I don't remember seeing this on 3.22

@rreckonerr
Copy link
Contributor

@sly7-7 I just symlinked 3.27.0-alpha.1 and it seems to work.

  • ember-qp-bug-reproduction output
    Package: ember-source
      * Specified: ~3.23.1
      * Symlinked: 3.27.0-alpha.1
  • yarn.lock contents in symlinked ember.js 3.27.0-alpha.1
    router_js@^7.1.1:
      version "7.1.1"
      resolved "https://registry.yarnpkg.com/router_js/-/router_js-7.1.1.tgz#cb7614a96cfb6bc65c066668b2dd32e3ad7ca38d"

Can you check out if it works for you following the steps below?

Steps to reproduce

  1. Clone ember.js repo
  2. git checkout v3.27.0-alpha.1
  3. yarn install
  4. yarn link
  5. Go to ember-qp-bug-reproduction folder and link yarn link ember-source
  6. yarn install --force
  7. ember s
  8. Check your browser

@sly7-7
Copy link
Contributor

sly7-7 commented Mar 2, 2021

@rreckonerr this is perfect timing :) I've just reproduced against 3.25 on a minimal repo. The bug seems to occur only if there is a loading template. I'm checking with this version right now

@sly7-7
Copy link
Contributor

sly7-7 commented Mar 2, 2021

@rreckonerr I've created https://github.com/sly7-7/bug-query-params where the bug is still here, even against ember's master branch.

@rreckonerr
Copy link
Contributor

rreckonerr commented Mar 2, 2021

@sly7-7 It might be that the fix is included in not yet release 3.27.x. 👇

Seems like this issue got fixed somewhere along the way. Tested it on 3.27.0 and here's the result

Were you able to fix the issue by linking the ember.js repo? (want to make sure that I didn't screw up something while doing so)

@sly7-7
Copy link
Contributor

sly7-7 commented Mar 2, 2021

@rreckonerr I've ran against master, so this should be the same result ? Anyway, I'll try :)

@rreckonerr
Copy link
Contributor

Appears that I was still running ember 3.25 even after yarn linking 3.27. The bug is still there, thanks @sly7-7

@sly7-7
Copy link
Contributor

sly7-7 commented Mar 4, 2021

@rreckonerr @patsy-issa , if that could help, as a workaround for now, I removed the loading template, and replace it by a loading action on the parent route which set a isLoading flag on the controller, coupled with an

 {{#if this.isLoading}}
   <LoadingComponent>
{{else}}
  {{outlet}}
{{/if}}

in the template.

sly7-7 added a commit to sly7-7/ember.js that referenced this issue Mar 4, 2021
sly7-7 added a commit to sly7-7/ember.js that referenced this issue Mar 4, 2021
sly7-7 added a commit to sly7-7/ember.js that referenced this issue Mar 4, 2021
@sly7-7
Copy link
Contributor

sly7-7 commented Mar 4, 2021

@patsy-issa This should be solved now, but we have to wait a new release of the router, and a bugfix release of ember.js

@rreckonerr
Copy link
Contributor

Yup, tested it against https://github.com/patsy-issa/ember-qp-bug-reproduction and it works!
Thanks, @sly7-7 🎉🎉🎉

rwjblue pushed a commit that referenced this issue Mar 8, 2021
(cherry picked from commit 7dbeded)
sly7-7 added a commit to sly7-7/ember.js that referenced this issue Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants