koajs / router

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

Unexpected behavior on nested routers #114

Open Alan-Liang opened 3 years ago

Alan-Liang commented 3 years ago

Hi. I noticed an inconsistent behavior between mounting a router onto an app (app.use(router.routes(), router.allowedMethods())) and onto another router (router.use('/base/path', subRouter.routes(), subRouter.allowedMethods())) when mounting happens before adding routes to the router. This is because when router.use() detects a sub-router it makes a clone of it instead of directly using it as a middleware:

https://github.com/koajs/router/blob/344ba0b323bdb78eefc800348577c6c67b99ea3c/lib/router.js#L264-L292

I wonder if this is really intentional.

@koa/router version: 10.0.0

Code sample:

const Koa = require('koa')
const Router = require('@koa/router')

const [ r1, r2, r3, r4 ] = Array(4).fill(0).map(() => new Router())

// correct behavior
const app = new Koa()
app.use(r1.routes(), r1.allowedMethods())
app.listen(3000)

r2.get('/status', ctx => ctx.body = { status: 0 })
r1.use('/api', r2.routes(), r2.allowedMethods())

// bad
const bad = new Koa()
bad.use(r3.routes(), r4.allowedMethods())
bad.listen(3001)

r3.use('/api', r4.routes(), r4.allowedMethods())
r4.get('/status', ctx => ctx.body = { status: 0 }) // disappears!

And then:

3imed-jaberi commented 3 years ago

@Alan-Liang, you cann't expect response from empty router (I mean router with out registred routes.)

const Koa = require('koa')
const Router = require('@koa/router')

const [r1, r2, r3, r4] = Array(4).fill(0).map(() => new Router())

// correct behavior
const app = new Koa()
app.use(r1.routes(), r1.allowedMethods())
app.listen(3000)

r2.get('/status', ctx => ctx.body = { status: 0 })
r1.use('/api', r2.routes(), r2.allowedMethods())

// bad
const bad = new Koa()
bad.use(r3.routes(), r4.allowedMethods())
bad.listen(3001)

// here i register the route for r4
r4.get('/status', ctx => ctx.body = { status: 0 })
// here i use the r4 routes as mw
r3.use('/api', r4.routes(), r4.allowedMethods())
jamesaspence commented 2 years ago

perhaps the docs just need to emphasize that routers are cloned at time of binding the middleware, but this feels like an intentional feature - I think it's reasonable to expect that a router would only apply what had already been registered at time of use.