Add optional parameter matching to $state.is and $state.includes

[roryf]

Fixes #413.

Allows passing a second parameter to $state.is and $state.includes with parameters for the state. is will do a strict angular.equals on the passed parameters with the current $stateParams, while includes will only attempt to match the parameters given.

Examples:

{ active: $state.is('site.section', {siteId: site.id, sectionId: section.id)} }

{ active: $state.includes('site.section', {sectionId: section.id)} }

I also changed the method contract so it won't return undefined when a state doesn't exist. This didn't make sense to me that a function could return a boolean value, or undefined, especially when the docs state it will return a boolean.

[nateabele]

Can you just add your tests, rather than removing existing tests? Also, please squash your commits when done. Thanks.

[roryf]

I had to change two existing tests because of the method contract change from undefined to false. Also one of the .includes() tests was incorrectly calling .is(). I changed the order so this is more obvious in the diff, and squashed the commits.

[timkindberg]

I feel like there was a reason for returning undefined that I can't remember now. It might have been for some other method though, like findState() and how it used to throw an error.

[nateabele]

@roryf Since we're talking about adding an extra method parameter, it should be easy to implement without a contract change. If you don't pass any parameters, the results should be identical to the preexisting behavior.

@timkindberg Yes, the return values do indeed have different semantic meanings: false means the state you're asking about doesn't match, but undefined means the state you're asking about doesn't exist.

[roryf]

@nateabele I agree it shouldn't break existing behaviour, so I'll change it back, but this seems like a big code smell to me.

[nateabele]

@roryf How so?

[roryf]

Perhaps "big" was an exaggeration 😄

A boolean method shouldn't return 3 possible states, by it's very nature of being boolean. The current behaviour muddies the water between two separate use cases: is foo the current state, and does the state foo exist. In the case of the former, if foo doesn't exist it's perfectly accurate to return false. The latter should be handled elsewhere, either through the current get method or a new exists method with a similar boolean contract.

This could be discussed in a separate issue, it doesn't need to affect the changes here.

[nateabele]

@roryf

A boolean method shouldn't return 3 possible states

Then maybe the doc comment needs an update?

This is JavaScript, not Java. 😉 Both values evaluate falsy, and the previous behavior was to simply throw an error, which everyone decided this was a much better alternative to.

[timkindberg]

I may actually agree with @roryf here. The methods are asking of the state "is?" or if it "includes?", these are yes/no questions. I'm not sure returning undefined would be used by anyone enough to really justify it.

I'm also fine with the way it was, returning undefined, but it does odd. @nateabele you yourself said you didn't like when things are used for more than one purpose, I think when we were discussing overloading of a function at some point. Returning undefined is similar to overloading.

However, I still have this feeling like there is some very specific reason why we need it to return undefined that probably only @ksperling knows.

[timkindberg]

Either way, it's merged, so nvm.

[nateabele]

@timkindberg

[Y]ou yourself said you didn't like when things are used for more than one purpose

Maybe you're thinking of when I've referenced the Single Responsibility Principle? I don't think I would have said anything that general, because it's not consistent with what I think. Consider jQuery.fn.val() or angular.module(). Both have more than one purpose.

However, I still have this feeling like there is some very specific reason why we need it to return undefined that probably only @ksperling knows.

That was the agreed-upon alternative to findState() throwing an error. There was no other reason.

[vdevappa]

Can someone please add support for the $ne operator for the JSON in Angular 1.x please?

{ active: $state.includes('site.section', {sectionId : { $ne: section.id } )} }

I have a situation where it needs to be active when it does not match a particular value.