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

Match routes strictly in order of definition (?) #97

Open
nikic opened this issue Mar 26, 2016 · 5 comments
Open

Match routes strictly in order of definition (?) #97

nikic opened this issue Mar 26, 2016 · 5 comments

Comments

@nikic
Copy link
Owner

nikic commented Mar 26, 2016

Currently the router uses a three tier fallback system:

  1. First match against provided method.
  2. If method is HEAD, match against GET routes.
  3. Match against * method routes.

It has been suggested that it may be preferable to avoid this kind of fallback behavior and instead match routes strictly in the order in which they were defined.

@Trainmaster
Copy link
Contributor

@nikic Can you or the one who suggested this elaborate a bit more on why this may be preferable?

@maaudet-ca
Copy link

I'm not 100% sure that I fully understand the points on discussion, but I'd say that if it matches 2 or more routes, it should either send an error, or send the list of matched routes and let the application decide the behavior. It shouldn't just fail silently and return the route that was the first to be defined.

@bwoebi
Copy link
Contributor

bwoebi commented Apr 4, 2016

@Manhim That it matches two routes is pretty normal, e.g. /foo/bar/{dynamic} and /foo/{dynamic}.
Failing upon two matching routes is not really an option. And as long as you don't want to use an exponential time algorithm to detect shadowing in regexes (via finite state machine reduction), we'll just have to live with the fact that routes might be completely shadowed...

@digitalnature
Copy link

digitalnature commented Apr 17, 2016

/{dynamic} and /foo throw an exception saying that the last one is shadowed.
I don't think this is a good idea. The dispatcher should just return a list of matched routes and let the handlers sort out which one should complete, because "shadowed" routes are pretty common on the web

@nikic
Copy link
Owner Author

nikic commented Apr 18, 2016

@Trainmaster The idea is to make the matching conceptually simpler (though the matching logic will be more complicated). Doing it this way would allow you to explain how routes are matched by just saying "in order of definition" and that's it. The current behavior has been unfavorably compared to the impenetrable CSS specificity rules ;)

@Manhim As Bob pointed out, it is very common to have multiple matching routes for a URL, typically some static route and a more general dynamic route. This is not an exceptional condition, it is perfectly ordinary. I don't think it would help anyone to return multiple routes in this case. The developer can decide which route will be returned based on the definition order (and we have warnings in place to catch obviously wrong ordering).

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

No branches or pull requests

5 participants