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

Cannot read property 'name' of undefined when trying to set queryParams in beforeModel #17118

Open
ohcibi opened this issue Oct 14, 2018 · 21 comments

Comments

@ohcibi
Copy link

ohcibi commented Oct 14, 2018

I'm getting the error TypeError: Cannot read property 'name' of undefined when trying to this.transitionTo({ queryParams: { somParam: 'someValue' } }) in this very line:

let leafRouteName = routeInfos[handlerInfoLength - 1].name;

I debugged it and it appears that routeInfos is an empty array and therefore that line must throw naturally. As I am unable to reproduce it in a twiddle and have no clue how to debug this further, I'd like to know what an empty routeInfos array actually means here and if this is an error after all or rather a bug? When looking at my app, it works as expected, i.e. the query parameter does get set and the URL in the browser is updated accordingly. So this is not an issue about the query parameter not being set.

For the matter of this issue, I'd say a check if that array is empty and a better error message should be added. I can't PR this as I don't understand the internals here, otherwise I would.

@pixelhandler
Copy link
Contributor

@ohcibi well if creating an example reproduction of the issue in a twiddle is not working out, perhaps create an example project and share the link to clone the repository.

@ohcibi
Copy link
Author

ohcibi commented Oct 19, 2018

I was able to do that in the meantime... I pushed it to https://github.com/ohcibi/erstifahrt and pushed the wip/activation-flash-message branch only but in case I messed up, check out that branch explicitly. Then simply npm install && npm start and open localhost:4200/anmeldung/1?activated=1. The app should redirect you to the same route without the query param but said error will still be thrown.

@dodeja
Copy link

dodeja commented Nov 13, 2018

Any update on this?

@ohcibi
Copy link
Author

ohcibi commented Nov 13, 2018

@dodeja do you have another example that differs from mine? If yes, please provide it as this issue seems to be more complicated than expected.

@ohcibi
Copy link
Author

ohcibi commented Nov 13, 2018

@pixelhandler have you been able looking into the example yet? Is it a valid example or is anything else needed?

@betocantu93
Copy link
Contributor

betocantu93 commented Nov 16, 2018

I stumbled with this too, I'm using ember-parachute

Facts I know:
This happens only when I immediately try to transitionTo after ember-simple-auth successfully login

login(credentials).then( () => 
  this.router.transitionTo('route.name', { queryParams: { userId: 5 } ) 
)

But I if I wrap the transitionTo with a

login(credentials).then( () => 
   setTimeout( () => //maybe a { next } from @ember/runloop might do it.
      this.router.transitionTo('route.name', { queryParams: { userId: 5 } ), 
   1000 )
)

It works... I know maybe the ember-parachute or ember-simple-auth has nothing to do, but I mean, maybe in that context it helps to debug, it's like a race condition

@betocantu93
Copy link
Contributor

betocantu93 commented Nov 16, 2018

It seems, ember-simple-auth UnauthenticatedRouteMixin is doing a transition for me as expected - if user is authenticated transitionTo('index') without any queryParams -

But at the same time I'm also triggering a transition to the same route but with queryParams (my app is served as an iframe, so external consumers send actions down to my ember app via postMessage API, and I can't control when they do it), and that's when I get this error.

If I instead try transitionTo another route, (not the same one) it works, without issue, as expected, it's like I need to wait to the router/transition to settle down, before I can attempt a new transition, to the same route.

I truly don't know about internals here, but it comes to my mind something like

this.router.hasSettled().then(() => this.router.transitionTo(...))

Or maybe its something that can be handled internally?

@jordpo
Copy link

jordpo commented Jan 31, 2019

I ran into this issue as well. The solution I came up with is to do a full transition.abort then retry inside a run.next block.

      next(this, function() {
        transition.abort();
        this.transitionTo(transition.to.name, {queryParams: validParams});
      });

@jordpo
Copy link

jordpo commented Jan 31, 2019

You probably don't want to do what I posted since it will trigger the transition.to's model hooks before retrying. I can confirm that this is only an issue when trying to transitionTo the same route but with different query params. It does look like it is supported per the docs:

https://guides.emberjs.com/release/routing/query-params/#toc_transitionto

// if you want to transition the query parameters without changing the route
this.transitionTo({ queryParams: { direction: 'asc' }});

Is exactly what I am trying to do. I'm on 3.7.x for ember.

@jordpo
Copy link

jordpo commented Jan 31, 2019

I can only replicate this actually when running tests. Calling visit('/') before visiting the route that has the query param transition does seem to fix it.

@Turbo87
Copy link
Member

Turbo87 commented Feb 3, 2019

@rwjblue any idea about this? I can repro this on one of my OSS apps too if someone needs it.

also, is this a duplicate of #14875?

@rwjblue
Copy link
Member

rwjblue commented Feb 3, 2019

I can only replicate this actually when running tests. Calling visit('/') before visiting the route that has the query param transition does seem to fix it.

So this error only occurs when doing a query param only transition during the initial visit?

@ohcibi
Copy link
Author

ohcibi commented Feb 3, 2019

@rwjblue when I created the issue the tasks I wanted to solve involved exactly that. If I recall correctly I even found out that this doesn’t happened when the page was loaded initially before that route was hit.

@barelyknown
Copy link

You can avoid the error by using replaceWith instead of transitionTo. This is probably the desired behavior anyhow in most situations. The only "catch" that I've noticed is that you must pass the route name to replaceWith (e.g. this.replaceWith('foo', { queryParams: { bar: 'baz' } })).

@Mciocca
Copy link

Mciocca commented Jul 24, 2019

I'm getting this error as well when trying to transition in a beforeModel hook. Using replaceWith instead of transitionTo still throws the error. I'm transitioning to the same route, only with added query parameters. Using Ember 3.5

@patrickberkeley
Copy link

FWIW this issue is still present in 3.12.0.

@rwjblue re your question:

So this error only occurs when doing a query param only transition during the initial visit?

Yes, that is correct.

@JackEllis
Copy link

@rwjblue Rob - My solution here: #14875 (comment)

Wanted to tag others in here because I ran into the solution.

@sly7-7
Copy link
Contributor

sly7-7 commented Mar 5, 2021

I was able to do that in the meantime... I pushed it to https://github.com/ohcibi/erstifahrt and pushed the wip/activation-flash-message branch only but in case I messed up, check out that branch explicitly. Then simply npm install && npm start and open localhost:4200/anmeldung/1?activated=1. The app should redirect you to the same route without the query param but said error will still be thrown.

@ohcibi In case you are still interrested, I've taken your example and updated it to run against ember.js and the router.js both on master branch. The app appears loading correctly, and the query params also seems correctly updated. But there was still an error in the console.
So, I've tried to take commits from tildeio/router.js#303 and all seem to be fine.

Screenshot from 2021-03-06 00-09-50

cc/ @rwjblue

@acorncom
Copy link
Contributor

acorncom commented Mar 8, 2021

@sly7-7 so from your testing it looks like tildeio/router.js#303 may fix the issue here if/when it gets merged?

@sly7-7
Copy link
Contributor

sly7-7 commented Mar 8, 2021

I would say 'maybe' actually, because there is an other similar issue here (with the same symptoms): #11563 in which it seems like tildeio/router.js#303 is not enough :(
But in any case we have to decide what to do with this router.js pr. And for now, I must admit I have no idea if this is good or not.

backspace added a commit to cardstack/cardstack that referenced this issue Jul 27, 2022
This is mostly a mechanical replacement of
transitionTo/transitionToRoute with this.router.transitionTo.

There’s a new test helper in web-client called visitWithQueryFix to
bypass failures described in this issue:
emberjs/ember.js#17118
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