refactor(ngRoute): make the order of route matching deterministic

[notNotDaniel]

Previously, routes were matched against the current $location in an arbitrary
order. This change ensures that routes are matched in the order they were
originally configured, making it possible to specify overlapping path
components which are fixed in some routes and variable in others. For example,

$route.when('/Book/new', ...);
$route.when('/Book/:bookId', ...);

can now be used to ensure that a different route configuration is used for
new books and for existing ones.

[mary-poppins]

Thanks for the PR!

• Contributor signed CLA now or in the past
  • If you just signed, leave a comment here with your real name
• PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

[lgalfaso]

Even when there is a value in being explicit on the order routes should be applied, this PR breaks the ability to redefine a route.
As a side note, all javascript implementations order the properties of objects in the order they were added, so in a strict (and not compliant way) the tests in the PR would pass in the current implementation

[notNotDaniel]

Lucas, thank you for your feedback. I was not aware that all javascript implementations consistently order properties in the order they were added. However, it would be nice to guarantee the order rather than counting on that behavior continuing.

I have updated the change to ensure that when a route is redefined, redundant entries are not added to the routeOrder array. On route redefinition the new routing information is still correctly associated with the path. I've added an additional test to check that route redefinition continues to work.

[lgalfaso]

Daniel, 7ac6627 works as intended, but having routes and routeOrder adds some redundancy. Do you think it might be possible to have it both ways?

[btford]

Is this a dupe of #3602 ?

[thenikso]

The problem with this solution to routing precedence is that writing whens does not (imo) explicitly make clear that the order is relevant.

@btford #3602 would solve a different problem where you may want to match:

$route.when('/Book/:bookId', ...);
$route.when('/Book/', ...);

currently, the second route would never be matched. This PR would solve the problem by inverting the order of those 2 whens. #3602 would solve it by not matching the first route if :bookId is empty.

[notNotDaniel]

Lucas, I considered using a single array to hold the routes, but that would have required searching the entire array for duplicates, which seemed less desirable than the redundancy of using two data structures. The redundancy is mitigated by the fact that that both routes and routeOrder are hidden from the end user, so one can not be modified without the other. However, I'd be happy to provide the array-only solution if that is deemed preferable.

[notNotDaniel]

@thenikso, the PR includes an update to the documentation to indicate that the order is significant and to give an example. As you said, #3602 changes the way routes are matched, whereas this PR only ensures that the order routes are checked is dependable.

[vojtajina]

I'm closing this PR, because:

• even with this change the order is still not deterministic, if you load modules asynchronously
• added tests pass without the code change (this is because JS - I believe all the implementations - DOES SORT properties based on the order of defining them)

Basically I think that this change does not solve anything.

@notNotDaniel Did you have any particular use case that didn't work for you and you were trying to fix it ? If so, please add a test for that use case.

[notNotDaniel]

• While it is worth noting that the order across modules would still not be deterministic (and I can add to the documentation to clarify that), this is still useful for the likely case where related routes (ones where order matters) are defined together, e.g. in the original example:

  $route.when('/Book/new', ...);
  $route.when('/Book/:bookId', ...);
  

  where the route for a new book needs to be matched before the route which shows the details of an existing book.

• The official documentation I could find (MDN and the EMCA-262 spec) explicitly states that the order for a for...in loop is arbitrary. I appreciate the fact that known current implementations do sort the properties on the order they were defined, and that being the case, this change is probably unnecessary. However, it seems prudent to err on the side of respecting the specification (particularly, I don't want to paint myself into a situation where I have to explain to my employer that something broke because I knowingly relied on implementation behavior which was explicitly not guaranteed by the standards).

I'll be happy to make the changes to ensure that this builds again if there's any chance it will be accepted.