QP only transition in beforeModel of application route throws error

[raytiley]

Reproduction: http://emberjs.jsbin.com/verocu/2/edit?html,js,output

Our app does some beforeModel checking of data to see what "location" the user is operating. If know query param for this is provided call transitionTo in order to force the user is at a location.

Because of some other logic I just added a catch to the promise chain that the transitionTo is a part of. Now I'm noticing the error shown in the above jsbin.

Interesting thing when trying to recreate is that if you have no catch on the promise chain and the QP is set refreshTrue then a second transition will fire, will have the correct query param and so the user won't notice the failure.

[mattmarcum]

👍 I created two failing tests for this

https://github.com/emberjs/ember.js/compare/master...mattmarcum:transition-to-in-before-model-with-query-params-test?expand=1

I went ahead and created failing tests in both the default code path and in the ember-routing-route-configured-query-params code path (which you can run in the tests by turning that feature flag on).

This also fails in the model hook, I've only tried those two hooks so far.

Anyone know whats up with that feature-flag? Its not in the FEATURES.md yet...

[mattmarcum]

This bug is pretty tricky because its caused by interaction between tildeo/router.js and ember.js/route.

When router.js is doing a queryParamTransition it generates an empty Transition because it should be a no-op transition. But ember.js/route needs to know the leaf node so that it can lookup the query params on the controller and do all its magic. When ember.js/route#finalizeQueryParamChange finally gets called, the transition passed to it contains no information about the current route, transition.state.handlerInfos is an empty array.

So it really seems like the fix should go in tildeo/router.js, probably around this line: https://github.com/tildeio/router.js/blob/master/lib/router/router.js#L126

If we did something like:

newTransition.state.handlerInfos = newState.handlerInfos;

I think that might fix it, but I'm not sure if that would break anything else;

[mattmarcum]

Actually that doesn't fix it. Even if you get the handlerInfos there's still some problems with getting the controller

[mattmarcum]

This is definitely caused by the fact that route.setup has not been called yet, it's not called until after the beforeModel hook. finalizeQueryParams expects that the route has already been setup.

Got to love the comments here: https://github.com/emberjs/ember.js/blob/v2.5.0/packages/ember-routing/lib/system/route.js#L1182-L1183

[rwjblue]

I can't tell if this is still an ongoing issue. Anyone have time to create a demo app or twiddle to confirm?

[buschtoens]

I just hit this as well in Ember 3.14.3, but I doubt that latest has fixed the bug. I believe that this is related to #12169.

I also believe that I found a "fix", but tbh I have no clue what's going on with router.js. 🤷‍♂️

- let newTransition = new InternalTransition(this, undefined, undefined);
+ let newTransition = new InternalTransition(this, undefined, newState);

https://github.com/tildeio/router.js/blob/d885da22340da98dcadb45427766efade54ce832/lib/router/router.ts#L123

[sly7-7]

@rwjblue I think this specific use case is working on the last release (3.25.1), because this looks a lot like this: #17494 (comment)

Update, seems like I spoke too fast. I've reproduced the use case of the original @raytiley example here: https://github.com/sly7-7/repro-ember-17494/tree/derived-from-ember-11563

It appears that on current release, the error is catched, but after closing the alert, the state seems good.
So I've tested with a potential fix (tildeio/router.js#303), and the error is still here, but the execution is the stopped.
In both cases, as noted by @raytiley, not catching the error will result at the end in a "good state", but the error shows in the console