ZijianHe / koa-router

Router middleware for koa.
MIT License
4.85k stars 408 forks source link

Generated match paths contain '//' (double slash), trailing slash or match everything #420

Open suricactus opened 6 years ago

suricactus commented 6 years ago

Hello!

I've found some issues with the generated paths of this module.

The example below is oversimplified, my routers are spread to many different files, so it is impossible to just stick to single router.

const Koa = require('koa');
const router = require('koa-router')();
const docsRouter = require('koa-router')();
const docTypesRouter = require('koa-router')();
const yetAnotherRouter = require('koa-router')();
const app = new Koa();

yetAnotherRouter.get('/something', (ctx) => { ctx.body = ctx._matchedRoute; })
docsRouter.get('/', (ctx) => { ctx.body = ctx._matchedRoute; });
docTypesRouter.use('/', yetAnotherRouter.routes()); // changes are applied on this line
docTypesRouter.get('/', (ctx) => { ctx.body = ctx._matchedRoute; });

router.use('/types', docTypesRouter.routes());
router.use('/', docsRouter.routes()); // changes are applied on this line

app.use(router.routes());

console.info(router.routes().router.stack.map(r => r.path))

app.listen(4565);
console.info('Now listening on http://localhost:4565/');

So the routes are [ '/types//something', '/types/', '//' ]. Why there is the double slash in the first route? Also why do we need the trailing slash in the second one? And look at the third one, I think it's not ok?

I tried to change all '/' to '' (empty string) on Router.use('', () => {}), but there is the trailing slash again: [ '/types/something', '/types/', '/' ]. This trailing slash is unnecessary, why the routes should be different if you add child routes or not?

Finally, I removed the path parameter from all Router.use(() => {}), and the result is totally different and I think totally wrong [ '/types(.*)/something', '/types/', '(.*)/' ], which matches whatever I pass to the url.

What I expected: 1) all scenarios above should produce the same paths to match. 2) no double slashes in the produced path 3) either all paths to have trailing slash, or not to have one

Am I missing something fundamentally?

jbielick commented 6 years ago

Finally, I removed the path parameter from all Router.use(() => {}), and the result is totally different and I think totally wrong [ '/types(.)/something', '/types/', '(.)/' ], which matches whatever I pass to the url.

This should be fixed in v7.4.0