-
Notifications
You must be signed in to change notification settings - Fork 38
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
Route (metadata) ordering is incorrect. #20
Comments
+1 |
As the original author of this code, I think I made a mistake in the original implementation. Unless I'm misunderstanding, I agree with this proposed change. |
This was referenced Aug 12, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The implementation of prioritising routes based on the specificity of their pattern introduced in e5047b8 is nice, but I believe the ordering is the reverse of what it should be.
I think if you have a mixture of static, dynamic and star components in the path the most specific path should be preferred (i.e.
/index.html > /:segment > /*path
). Simple cases like this work (as does the test case provided in the referenced commit) but not because of the ordering conditions but rather because in this simple case the higher priority paths contain none of the lower priority components.Consider instead
/archive/*path
vs/*path
and you see that the ordering constraints as currently implemented do the reverse in that/*path
(the least static) is the preferred path.I believe instead the ordering should be as follows:
The ordering of the final
stars
comparison could go either way. As I have proposed it here, the path/*pre/and_then/*post
is preferred over/archive/*path
because the latter is least specific (containing only two components versus three). An alternative would be to ignore the number of stars in the ordering and fall back to the default ordering so long as it is deterministic.I believe the default ordering currently is such that if two matches are equal (based on metadata ordering) then the first defined is chosen (please correct me if I am wrong). A deterministic default ordering (or else an explicit one) is required because I don't believe there is any sensible way to automatically decide between
/archive/*path
and/*path/meta
The text was updated successfully, but these errors were encountered: