koajs / router

Router middleware for Koa. Maintained by @forwardemail and @ladjs.
MIT License
853 stars 181 forks source link

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

Closed alecmev closed 2 years ago

alecmev commented 3 years ago

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 commented 2 years ago

PR welcome?

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

etroynov commented 2 years ago

@3imed-jaberi, @alecmev can you look at it? https://github.com/koajs/router/pull/144