Skip to content

fix(common.js): Error if parents[i] is undefined #1975

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

Closed
wants to merge 1 commit into from
Closed

fix(common.js): Error if parents[i] is undefined #1975

wants to merge 1 commit into from

Conversation

AlexJuarez
Copy link

hasOwnProperty still would throw an error if parents[i] was undefined.

@eddiemonge
Copy link
Contributor

What case would parents[i] be undefined since var i in parents?

@AlexJuarez
Copy link
Author

The parents array can be populated with undefined, right now I'm working on a app that im switching from ember router to angular ui router, and the ember router pushes undefined into the array.

@eddiemonge
Copy link
Contributor

That sounds like a bug that needs to be fixed there but having the check here isn't bad. The check itself could be improved though. I think it should be something like parents[i] && !parents[i].params.

@AlexJuarez
Copy link
Author

Changing this line allows both routers to co-exist

@AlexJuarez
Copy link
Author

What about typeof parents[i] === 'undefined' || !parents[i].params

@AlexJuarez
Copy link
Author

I would still like to get this fix in, even though its low priority.

@AlexJuarez AlexJuarez changed the title fix(common.js): this can error if parents[i] is undefined fix(common.js): Error if parents[i] is undefined May 27, 2015
@christopherthielen
Copy link
Contributor

I'm ok with merging this in if we get a unit test, but ...

Changing this line allows both routers to co-exist

...is obviously not a goal of this project

@christopherthielen
Copy link
Contributor

Whoops, looks like #2478 was an exact dupe of this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants