fix: qp-only transition during initial transition #307
Conversation
|
Please see my comment here: emberjs/ember.js#18577 (comment) Not sure if this is another bug or if this fix does not address the root cause. |
| @@ -120,7 +120,7 @@ export default abstract class Router<T extends Route> { | |||
| // perform a URL update at the end. This gives | |||
| // the user the ability to set the url update | |||
| // method (default is replaceState). | |||
| let newTransition = new InternalTransition(this, undefined, undefined); | |||
| let newTransition = new InternalTransition(this, undefined, newState); | |||
| newTransition.queryParamsOnly = true; | |||
There was a problem hiding this comment.
| newTransition.queryParamsOnly = true; | |
| newTransition.queryParamsOnly = true; | |
| this.setupContexts(newState, newTransition); |
Some other tests started breaking with the undefined -> newState change in L123, because this apparently leaks through Routes which haven't yet been passed through setupContexts(...) and thus did not have their setup(...) hook called.
Ultimately this causes Route#finalizeQueryParamChange to fail, as it expects route.controller to be defined.
|
This fix looks good conceptually, but we need to fixup the tests and add a specific test for the scenario described in emberjs/ember.js#18577 |
|
@buschtoens status? |
|
@buschtoens @rwjblue Is this solution still good? if so is there anything i can do to help get this merged? Happy to write the test for it if i can be pushed in the correct general direction, im not overly familiar with this code. |
Seems to fix emberjs/ember.js#18577.