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

{{link-to}} with model and query params and target route with refreshModel: true errors #13395

Closed
wagenet opened this issue Apr 22, 2016 · 7 comments

Comments

@wagenet
Copy link
Member

wagenet commented Apr 22, 2016

https://ember-twiddle.com/c7cd718c4960e8a5d6b3abe3f2de5fd3 (Click link and see console)

wagenet referenced this issue Apr 22, 2016
Refreshing the route heighercy do to query param change on the initial transition is pointless, so now we only call refresh if there is a `currentState`. This fixes #10945

(cherry picked from commit 92cd0a2)
@wagenet
Copy link
Member Author

wagenet commented Apr 22, 2016

A possible fix is to check in queryParamsDidChange that the route is actually active. AFAICT the problem happens because it fires (unncessarily?) before the route is actually entered.

@raytiley
Copy link
Contributor

This seems the same as an issue I'm trying to trackdown:

https://ember-twiddle.com/0c7e72433e1a3149a7304007fbbd1a54

The issue, as best I can explain it, is that queryParams only transitions have a special short circuit in router.js that fires the queryParamsDidChange https://github.com/tildeio/router.js/blob/master/lib/router/router.js#L38-L43

This code can get hit before the contexts for a route are setup yet, so there is not controller to set the query params on. There must be some code path, maybe setupContexts that gets fired in a normal transition after that short circuit for the queryParams only transition. I spent a few hours digging in the other night to get this far, hoping I can figure it out further over the weekend /CC @rwjblue (this is the crazy shit I wanted to pair w/ you a bit on)

@wagenet
Copy link
Member Author

wagenet commented Apr 22, 2016

To be clear, the issue appears to not just be for queryParam only transitions. It also happens when you're transitioning to a new route with query params.

@mattmarcum
Copy link

This looks to be the same bug as this: #11563.

@raytiley I came to the same conclusion about the controller not being setup yet. Any chance your wip branch can be extended to cover this bug too? I have a branch with the failing tests here

@Serabe
Copy link
Member

Serabe commented Jul 13, 2017

More reproductions of this issues can be found in #13954

@pixelhandler
Copy link
Contributor

@mattmarcum @raytiley @wagenet is this still an issue, perhaps we should close or create a new reproduction of this, what do you think?

@pixelhandler
Copy link
Contributor

Closing for now, feel free to re-open if you can reproduce in the current release of Ember.

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

5 participants