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

Unexpected behavior on nested routers #114

Open
Alan-Liang opened this issue Jan 24, 2021 · 2 comments
Open

Unexpected behavior on nested routers #114

Alan-Liang opened this issue Jan 24, 2021 · 2 comments

Comments

@Alan-Liang
Copy link

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:

router/lib/router.js

Lines 264 to 292 in 344ba0b

if (m.router) {
const cloneRouter = Object.assign(Object.create(Router.prototype), m.router, {
stack: m.router.stack.slice(0)
});
for (let j = 0; j < cloneRouter.stack.length; j++) {
const nestedLayer = cloneRouter.stack[j];
const cloneLayer = Object.assign(
Object.create(Layer.prototype),
nestedLayer
);
if (path) cloneLayer.setPrefix(path);
if (router.opts.prefix) cloneLayer.setPrefix(router.opts.prefix);
router.stack.push(cloneLayer);
cloneRouter.stack[j] = cloneLayer;
}
if (router.params) {
function setRouterParams(paramArr) {
const routerParams = paramArr;
for (let j = 0; j < routerParams.length; j++) {
const key = routerParams[j];
cloneRouter.param(key, router.params[key]);
}
}
setRouterParams(Object.keys(router.params));
}
} else {

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:

  • http://localhost:3000/api/status => 200 {"status":0}
  • http://localhost:3001/api/status => 404 Not Found
@3imed-jaberi
Copy link
Member

@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
Copy link

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.

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

3 participants