koajs / router

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

Fix problems caused by routerPath #144

Closed cooperhsiung closed 2 years ago

cooperhsiung commented 2 years ago

ctx.params is unexpectedly affected by other routers

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

const app = new Koa();
const router1 = new Router();
const router2 = new Router();

router1.get('/users/:userId', async (ctx, next) => {
  console.log(ctx.params) // got { userId: '123' }
  await next();
});

router2.get('/users/:id', async (ctx, next) => {
  console.log(ctx.params) // got { userId: '123', id: ':userId' }
  ctx.body = 'hello, user is ' + ctx.params.id;
});

app.use(router1.routes());
app.use(router2.routes());

app.listen(3000);

send a request

const axios = require('axios');

axios
  .get('http://127.0.0.1:3000/users/123')
  .then((ret) => {
    console.log(ret.data);  // got hello, user is :userId
  })
  .catch((err) => {
    console.error(err);
  });

what is expected is hello, user is 123, instead of hello, user is :userId


I found that the problem stemmed from this line:

https://github.com/koajs/router/blob/1aead99e0e0fdb8666e9c6fa2f52b0463c622025/lib/router.js#L366

It seems that it has caused a series of problems

originnal feature imports

unexpected code import

related pull requests

related issues


ctx.params shouldn't have been affected by other routers, this looks like a bug that needs to be fixed for this essential module of the koa ecosystem.

In order not to break compatibility, I choose to restore ctx.routerPath after layers match and added corresponding tests

etroynov commented 2 years ago

@3imed-jaberi, @alecmev can you look at it?

titanism commented 2 years ago

We have just published @koajs/router v11.0.0 which resolves this issue. This is mirrored to koa-router as well.

https://github.com/koajs/router/releases/tag/v11.0.0

This project is maintained by Forward Email and Lad.