fix(ngRoute): dont duplicate optional params into query

[omsmith]

When calling updateParams with properties which were optional, but
previously undefined, they would be duplicated into the query params as
well as into the path.

jsfiddle reproduction

From #updateParams docs:

Provided property names that match the route's path segment definitions will be interpolated into the location's path, while remaining properties will be treated as query params.

In the actual implementation of updateParams the "remaining properties" are determined prior to interpolation by checking if the pathParam for the given key has a value; however, optional parameters may not exist in pathParams.

Alternative fixes might include

• (probably better) running interpolate before separating searchParams, since it will remove matched keys from the passed in newParams object.
• (maybe more correct?) include undefined optional params in the pathParams object (switchRouteMatcher L536, if (key && val) -> if(key)) and then check for the property as opposed to truthiness

[googlebot]

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

[shahata]

This sounds reasonable to me but I think the implementation can be much more simple:

updateParams: function(newParams) {
  if (this.current && this.current.$$route) {
    newParams = angular.extend({}, this.current.params, newParams);
    $location.path(interpolate(this.current.$$route.originalPath, newParams))
      .search(newParams);
  }
  else {
    throw $routeMinErr('norout', 'Tried updating route when with no current route');
  }
}

[omsmith]

Corporate CLA just sent over (D2L Corporation)

[omsmith]

@shahata Yeah that was my first alternative suggestion - I'll push the change

[omsmith]

We sent over details about the new Google Group based CLA authorization on Monday. Haven't heard back, but maybe we're good to go?

[petebacondarwin]

@shahata - can you check with the guys in the office that the CLA is good then look at merging this when you are happy with it?

[omsmith]

@shahata @petebacondarwin Alright, actually got the e-mail back now, CLA should be good to go

[googlebot]

CLAs look good, thanks!