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

The matchedRoute fix is a breaking change and should be documented #126

Closed
alecmev opened this issue Aug 18, 2021 · 2 comments
Closed

The matchedRoute fix is a breaking change and should be documented #126

alecmev opened this issue Aug 18, 2021 · 2 comments

Comments

@alecmev
Copy link

alecmev commented Aug 18, 2021

I'm talking about #93. Specifically, the addition of ctx.routerPath = layer.path.

I understand that this was unintended behavior, but it has existed for multiple major versions, from 5.2.x to 9.0.x, spanning 5 (!) years, and I imagine many relied on it, like ourselves, so it's certainly a breaking change and warranted a major release. I've upgraded our relatively convoluted Koa backend recently from 7.x to 9.x, and it took a few hours to finally understand that the problems we were having weren't caused by the update of path-to-regexp, but by this innocuous-looking PR.

Please, mention this in history.md, at very least. Would also be nice to know how to get the "fall-through" behavior back without patching @koa/router in node_modules, since there's nothing inherently wrong about it.

It also seems a bit off that two separate router instances can affect each other, like seen in #101. Is that not a bug?

Off-topic: No offense to the creators and the maintainers, I do appreciate the effort and I got great value out of this library, but koa-router has been giving me much more trouble than koa-tree-router over the years, and I feel like it doesn't deserve to be the @koa/router without stepping up its game.

@niftylettuce
Copy link
Contributor

PR welcome?

Thanks for sharing koa-tree-router, will check it out.

@etroynov
Copy link
Contributor

etroynov commented Jun 26, 2022

@3imed-jaberi, @alecmev can you look at it?
#144

@titanism titanism closed this as completed Jul 4, 2022
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

4 participants