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

Use longest match for ctx._matchedRoute, fixes #478 #492

Closed
wants to merge 1 commit into from

Conversation

jcao219
Copy link

@jcao219 jcao219 commented Jan 12, 2019

Fixes #478 by using the longest path, or the length of path string as a tiebreaker to determine "most specific layer"

I tried to implement it without replacing RegExp.prototype.test with something that like RegExp.prototype.exec which returns more match information, as that would decrease routing performance.

This does subtly change the behavior of koa-router, though. Is 5.x release planned soon, and will it fix #478 ?

Use most recently added layer as a tiebreaker
@mirague
Copy link

mirague commented Jan 29, 2019

Was running into this issue in needing to access the proper path pattern for the current route handler. Silly (useless) that it uses the first "parent" route path rather the more specific route handler.

zacanger added a commit to zacanger/koa-modern-router that referenced this pull request Feb 16, 2019
Apply changes from ZijianHe/koa-router#492
One test failing, will address after
@falkenhawk
Copy link

@jaco219 this repo is abandoned. Maybe you'd be interested to contribute to the active repo instead: koajs/router#34 (comment)

@jcao219
Copy link
Author

jcao219 commented Jun 30, 2020

Let's close the issue. Feel free to merge my code into any forked repo if you feel like it.

From the time I made this PR up until now, I've moved across continents twice. I also use fastify now (as my framework of choice). I haven't thought about Koa router in 12 whole months.

@jcao219 jcao219 closed this Jun 30, 2020
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

Successfully merging this pull request may close these issues.

._matchedRoute[Name] inconsistency with nested routes.
3 participants