Redirecting a transition to same route with different query params does not work unless transition is aborted before

[jelhan]

I'm seeing two weird bugs if doing a redirect to the same route with different queryParams in beforeModel hook:

1. If the route is entered via <LinkTo> or transitionTo(), the query param is not reflected in the URL.
2. If the route is entered directly on application start, ember throws an error.

The error thrown is this one:

TypeError: Cannot read property 'name' of undefined
    at Class._queryParamsFor (https://t95pf.sse.codesandbox.io/assets/vendor.js:35740:59)
    at Class.finalizeQueryParamChange (https://t95pf.sse.codesandbox.io/assets/vendor.js:34759:29)
    at Class.triggerEvent (https://t95pf.sse.codesandbox.io/assets/vendor.js:36198:27)
    at PrivateRouter.triggerEvent (https://t95pf.sse.codesandbox.io/assets/vendor.js:35078:43)
    at PrivateRouter.finalizeQueryParamChange (https://t95pf.sse.codesandbox.io/assets/vendor.js:72248:12)
    at PrivateRouter.queryParamsTransition (https://t95pf.sse.codesandbox.io/assets/vendor.js:71741:37)
    at PrivateRouter.getTransitionByIntent (https://t95pf.sse.codesandbox.io/assets/vendor.js:71810:37)
    at PrivateRouter.transitionByIntent (https://t95pf.sse.codesandbox.io/assets/vendor.js:71759:21)
    at PrivateRouter.doTransition (https://t95pf.sse.codesandbox.io/assets/vendor.js:71894:19)
    at PrivateRouter.transitionTo (https://t95pf.sse.codesandbox.io/assets/vendor.js:72372:19) 

It's caused by routeInfos of transition being an empty array here: https://github.com/emberjs/ember.js/blob/v3.14.1/packages/%40ember/-internals/routing/lib/system/route.ts#L2523 _queryParamsFor can't handle that case and throws at this line: https://github.com/emberjs/ember.js/blob/v3.14.1/packages/%40ember/-internals/routing/lib/system/router.ts#L926

For a minimal reproduction use this code:

// app/routes/foo.js

import Route from '@ember/routing/route';

export default Route.extend({
  async beforeModel(transition) {
    if (!transition.to.queryParams.bar) {
      await this.transitionTo('foo', { queryParams: { bar: "1" }});
    }
  }
});

// app/controllers/foo.js

import Controller from '@ember/controller';
import { inject as service } from '@ember/service';

export default Controller.extend({
  router: service(),

  queryParams: ['bar'],
});

// app/templates/foo.hbs

{{log router.currentRoute}}

The current route is logged in template to verify that the query params are available on RouteInfo object for scenario 1. And they are indeed. They are just not represented in URL.

Both bugs could be fixed by explicitly aborting the transition before doing the redirect:

// app/routes/foo.js

import Route from '@ember/routing/route';

export default Route.extend({
  async beforeModel(transition) {
    if (!transition.to.queryParams.bar) {
      transition.abort();

      await this.transitionTo('foo', { queryParams: { bar: "1" }});
    }
  }
});

But accordingly to documentation transitionTo() should implicitly abort the transition. It should not be required to do it manually. Also if this is the recommended solution, there should be an assertion if the transition is not aborted before. It should not end with a broken state (URL not reflecting query parameters in URL) neither with throwing a TypeError.

There are some issues which seem to be related but slightly different:

• transition.abort() does not work for query param only transitions #18568
• Cannot read property 'name' of undefined when trying to set queryParams in beforeModel #17118
• replaceWith / transitionTo doesn't respect query params on beforeModel #14606
• QP only transition in beforeModel of application route throws error #11563
• Query params not updated by transitionTo/replaceWith during active transition #10262

[acorncom]

@jelhan which version of Ember are you seeing this in?

[jelhan]

@acorncom I'm quite sure that I had reproduced it using the latest version of Ember at the same day when reporting. So I think it was 3.14.1. But I was also able to reproduce it with latest ember-source available today, which is 3.16.0.

I created a repository with the reproduction mentioned above: https://github.com/jelhan/ember-demo-redirect-to-same-route-with-different-query-params

Here are the steps to reproduce both issues mentioned above using that repo. Assuming that you have cloned it, installed dependencies and run ember serve.

1. transition to route triggering the redirect from another route

• Open http://localhost:4200
• Click on the link

Expected:

• URL should be http://localhost:4200/foo?bar=1
• currentURL property of RouterService should be /foo?bar=1
• currentRoute.queryParams should deep equal { bar: 1 }

Actual:

• URL is http://localhost:4200/foo (missing query param)
• currentURL property of RouterService is /foo (missing query param)
• currentRoute.queryParams deep equals { bar: 1 } (correct but out of sync with URL)

2. open that route directly

• Open http://localhost:4200/foo

See an error been thrown and nothing rendered. The error is:

TypeError: routeInfos[(routeInfoLength - 1)] is undefined

It's caused by routeInfos of transition being an empty array here: https://github.com/emberjs/ember.js/blob/v3.14.1/packages/%40ember/-internals/routing/lib/system/route.ts#L2523 _queryParamsFor can't handle that case and throws at this line: https://github.com/emberjs/ember.js/blob/v3.14.1/packages/%40ember/-internals/routing/lib/system/router.ts#L926

Fixing the error by aborting the transition

To reproduce that aborting the transition before doing the redirect fixes both issues uncomment these line: https://github.com/jelhan/ember-demo-redirect-to-same-route-with-different-query-params/blob/1326804684273b9369725099f36f262fabdd9832/app/routes/foo.js#L6-L7

[chriskrycho]

I have what appears to be an additional variant of this, available in this repo.

If you try to do a transitionTo or replaceWith in a route's afterModel, and it's a query-params-only transition, and it's the first time entering the application (i.e. first render), it will fail with exactly the error @jelhan reported above. Moreover, transition.abort() does not fix things in this case; it just leaves you with a dead app. 😬

To see the reproduction behavior and how it varies for later entries, comment out these lines: later transitions work fine; only the first transition into the route fails this way. To see that transition.abort() only further hoses things in this case, comment those lines back out and uncomment this line.

[xg-wang]

@chriskrycho I think I just ran into the bug you're describing, but want to confirm.
I'm seeing 404 when clicking your link. Do you still have the repo somewhere?

[chriskrycho]

Fixed! I accidentally made a private repo. Should be visible now!

[snewcomer]

Same...Here is my repo.

beforeModel() {
  // flawed logic through a refactor that eventually lead to this
  // route was "a-route.index"
  this.router.transitionTo(sameRouteAsIAmOn);
}

[buschtoens]

Cross-posting #11563 (comment):

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);

tildeio/router.js@d885da2/lib/router/router.ts#L123

[jelhan]

I updated my reproduction repository to Ember 3.22. Can still reproduce the bug with that latest stable release.

The error message thrown in the second case changed a little bit. It's now: TypeError: Cannot read property 'name' of undefined. But I didn't noticed any change of the actual situation. Maybe Chrome just changed the behavior for array[-1]?

The full stack trace to help people finding this issue with Google:

Error while processing route: foo Cannot read property 'name' of undefined TypeError: Cannot read property 'name' of undefined
    at Router._queryParamsFor (http://localhost:4200/assets/vendor.js:23164:59)
    at Class.finalizeQueryParamChange (http://localhost:4200/assets/vendor.js:22188:29)
    at Router.triggerEvent (http://localhost:4200/assets/vendor.js:23622:27)
    at PrivateRouter.triggerEvent (http://localhost:4200/assets/vendor.js:22506:43)
    at PrivateRouter.finalizeQueryParamChange (http://localhost:4200/assets/vendor.js:63090:12)
    at PrivateRouter.queryParamsTransition (http://localhost:4200/assets/vendor.js:62580:37)
    at PrivateRouter.getTransitionByIntent (http://localhost:4200/assets/vendor.js:62652:37)
    at PrivateRouter.transitionByIntent (http://localhost:4200/assets/vendor.js:62601:21)
    at PrivateRouter.doTransition (http://localhost:4200/assets/vendor.js:62736:19)
    at PrivateRouter.transitionTo (http://localhost:4200/assets/vendor.js:63214:19)

I applied @buschtoens patch. This resolves the original bug but reveals another one: The to property of transition passed as first argument to beforeModel hook is undefined.

[rreckonerr]

@jelhan I've seen the same error while working on an issue related to disappearing QP's!
Can you please check if @buschtoens's tildeio/router.js#307 and mine tildeio/router.js#308 combined solve the issue? I believe it might help since we both fix changes brought in by the same commit 🤔🤔🤔.

[jelhan]

@rreckonerr If I got it right you suspect that the bug was introduced by tildeio/router.js@f385d11. It was released in [email protected].

Ember.js upgraded to router_js@^5 here: #17007

[email protected] is the first stable release containing this version:

ember.js/package.json

Line 149 in a5f870f

"router_js": "^6.1.3",

The stable release before [email protected] as using [email protected]:

ember.js/package.json

Line 139 in abf753a

"router_js": "2.0.0-beta.4",

Will try to find some time to verify that the bug is reproducible with [email protected] but not with [email protected]. The [email protected] contains many router related changes. But at least it would help to trace down the bug to that big router refactoring.

[kmccullough]

I am experiencing the same issue where an exception is thrown when the application is loaded on the route which is transitioning only to change the query-params. It's a very simple reproduction in 3.22: https://github.com/kmccullough/ember-default-query-param

[sly7-7]

@kmccullough Thanks for this example :)
Locally, with the recent changes on the router, I don't encounter your error anymore. :)

@jelhan There is no more error with the last changes, but the behavior is still inccorrect, I mean the qp update is not reflected in the URL. I'll try to investigate. Thanks to the works done by @alexlafroscia and @buschtoens maybe this will be resolved.

[patrickberkeley]

I'm seeing this in ember 3.26.1 as well.

[panthony]

Same problem as @chriskrycho with ember 3.22.

Also cannot use transition.abort() because it breaks the app when it's a first render.

[YoranBrondsema]

We're still seeing this issue in Ember.js 5.1.

If we're replacing a route with the same route but different query params:

export default class PortfolioRoute extends Route {
    beforeModel() {
        this.router.replaceWith('portfolio', {
          queryParams: {
            bla: 'bla'
          }
      });
    }
}

the route crashes on first render with:

router.js:836 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'name')
    at Router._queryParamsFor (router.js:836:1)
    at ApplicationRoute.finalizeQueryParamChange (route.js:1242:1)
    at Router.triggerEvent (router.js:1219:1)
    at PrivateRouter.triggerEvent (router.js:234:1)
    at PrivateRouter.finalizeQueryParamChange (router_js.js:1703:1)
    at PrivateRouter.queryParamsTransition (router_js.js:1257:1)
    at PrivateRouter.getTransitionByIntent (router_js.js:1317:1)
    at PrivateRouter.transitionByIntent (router_js.js:1275:1)
    at PrivateRouter.doTransition (router_js.js:1399:1)
    at PrivateRouter.transitionTo (router_js.js:1809:1)

Unfortunately none of the workarounds seem to work. Any suggestions on getting around the bug?

[rassloff]

same problem in my project. any suggestions ???

[seanCodes]

Just encountered this on Ember 4.12. 😞 @buschtoens fix in tildeio/router.js#307 does seem to fix it for me, though. Pretty amazed this has gone so long without being addressed since it seems like a common scenario.

Anyone know of any workarounds or any way this could be monkey-patched?

[Mikek2252]

@seanCodes I have been investigating this bug and also found that if i was trying to transition multiple times it would also trigger e.g.

beforeModel() {
  if (foo) {
    this.transitionTo('this.route', { queryParams });
  }
  if (bar) {
    this.transitionTo('this.route', { queryParams });
  }
}

By adding returning on transition it stopped the error.

beforeModel() {
  if (foo) {
    return this.transitionTo('this.route', { queryParams });
  }
  if (bar) {
    return this.transitionTo('this.route', { queryParams });
  }
}

Maybe this might help in your case?