ZijianHe / koa-router

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

Issue accessing route params with prefix #422

Open amangeot opened 6 years ago

amangeot commented 6 years ago

Hello, I encounter issues accessing route params from prefixed routers, here is the code at fault:

new Router({ prefix: '/users/:id' })
 .param('id',  (id, ctx, next) => { console.log(id) })

id is undefined.

Is there a way to achieve this?

spanitz commented 6 years ago

Got the same issue while migrating from 5.2.3 to 7.3.0. In some cases the param will be passed, in others not. ~Seems kind of random...~

spanitz commented 6 years ago

@amangeot It turns out that my issue is similar to yours, but not exactly the same. Note that the param middleware behaves different in several scenarios. So, here's a snippet, to reproduce and illustrate the issue (tested with Node 9.4.0).

const Koa = require('koa');
const Router = require('koa-router');

const app = new Koa();

const router = new Router({ prefix: '/foo/:bar' })
    .param('bar', (bar, ctx, next) => {
        // 1. this function will be called for each router middleware on the stack
        // in this case two times. but bar is defined only once. I guess before the
        // verb stack (GET) gets called.

        // 2. this function won't be called on OPTIONS preflights
        ctx.bar = bar;
        return next();
    })
    .use(async (ctx, next) => {
        // ctx.bar is undefined at this point
        // with koa-router@5.2.3 `bar` is assigned with the correct value in 
        // the generator functions context (`this.bar`)
        await next();
    })
    .get('/baz', async (ctx, next) => {
        ctx.body = ctx.bar;
        await next();
    });

app.use(router.routes());
app.use(router.allowedMethods());

app.listen(3000);

Can anyone explain why this behavior has changed that way? I guess a lot of people working with use() to access and mutate context before the verbs come in to play. As well as some assume that param() has high priority on the stack.

amangeot commented 6 years ago

I have a similar use case, the nested router's middlewares run before param(). It looks like param() is called when the route is matched hence after the middlewares

jbielick commented 6 years ago

I think this might be because of:

https://github.com/alexmingoia/koa-router/blob/071b26489cb5c1886b2f89894a4633b18e9822de/lib/router.js#L276

Where !hasPath is truthy because app.use(() => has no prefix (path). When the code later gets here during a route match:

https://github.com/alexmingoia/koa-router/blob/071b26489cb5c1886b2f89894a4633b18e9822de/lib/layer.js#L95-L96

I think the captures are ignored here. I have a failing test locally case showing this.

I don't think the order of param / generic middleware execution changed between those versions—I've been comparing them and haven't found differences yet and I'm the one responsible for changes to dispatch between 5.x and 7.x

https://github.com/alexmingoia/koa-router/blob/071b26489cb5c1886b2f89894a4633b18e9822de/lib/router.js#L334-L351

divmgl commented 5 years ago

Any update on this? I just ran into this issue.

domosapien commented 5 years ago

I ran into this as well. I reached into the stack and set the ignoreCaptures to false after I defined my middleware, and I got my parameter in the ctx.params.

I was originally trying to use the router.param method to get the param out, but was having issues with that running unless the router was set up to use a prefix vs having the route defined when it was attached. Switching to the use instead of param allows me to fix this setting and not need the prefix defined.