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

onEnter is called twice if a parameter is an array with more than one element #1414

Closed
realityking opened this issue Sep 29, 2014 · 9 comments
Assignees
Labels
Milestone

Comments

@realityking
Copy link

This seems to be a regression and currently stops us from updating to 0.2.11.

Plunkr: http://plnkr.co/edit/PmwRrdSMnlAetItQBeBl?p=info

The issue only occurs und the following conditions:

  1. The parameter is a query parameter
  2. The parameter is an array
  3. The array contains more than one element

If you need any more information I'd be happy to help. I digging trough the callstack for both calls, and they seem to come form completely different places. I'm not familiar enough with the inner workings of UI Router to make any sense out of the behavior I saw.

@christopherthielen
Copy link
Contributor

You're seeing this:
* BREAKING CHANGE: state parameters are no longer automatically coerced to strings, and unspecified parameter values are now set to undefined rather than null.


The params code got a major overhaul in 0.2.11. You were relying on the array being coerced to a String.

You can now implement your own typed params, and could create an array type.
http://angular-ui.github.io/ui-router/site/#/api/ui.router.util.$urlMatcherFactory
http://angular-ui.github.io/ui-router/site/#/api/ui.router.util.type:UrlMatcher
http://angular-ui.github.io/ui-router/site/#/api/ui.router.util.type:Type

However, from what I can tell, it looks like query search params can not be typed. Hmmm... @nateabele is this by design?

I need to do some more research, stay tuned. https://github.com/angular-ui/ui-router/blob/master/src/urlMatcherFactory.js#L143

@christopherthielen christopherthielen self-assigned this Oct 1, 2014
@nateabele
Copy link
Contributor

@christopherthielen Nope. Weird. Btw, I had a quick poke at this. It looks like the whole resolved promise actually gets resolved twice. Sigh... soon.

@christopherthielen
Copy link
Contributor

I'll take this, I need to get familiar with the typed params anyway.

@christopherthielen christopherthielen added this to the 0.2.12 milestone Oct 1, 2014
@christopherthielen
Copy link
Contributor

I've got a bunch of Type fixes in a private branch. Here's a working plunk.

http://plnkr.co/edit/KQ9PdnouwJskxY4IK7uO?p=preview

@nateabele I have about 6 commits to review with you. You can see the cumulative codebase with all the commits at https://github.com/christopherthielen/ui-router/tree/default-type-coersion

@realityking
Copy link
Author

Anything I could do to help move this along?

@christopherthielen
Copy link
Contributor

@realityking, thanks for the offer . I have a few more things to clean up related to Type(s) before I push this. 0.2.12 should come shortly after that.

@realityking
Copy link
Author

Alright. I'll test this in our app after this is merged (and hopefully before the release).

@christopherthielen
Copy link
Contributor

@realityking please build from master and test

http://plnkr.co/edit/FVt7dokpRZDgd6wpn6ee?p=preview

@realityking
Copy link
Author

Works perfectly. Thank you!

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

No branches or pull requests

3 participants