fix: other routers except the first one fails to match in some scenes

1 Problem

In some cases, when defining multi routers, only the first router get matched, the others are invalid.

Code to confirm problem:

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

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

router1.all(/.*/, async (ctx, next) => {
    console.log(ctx.path);
    console.log('match anything');
    await next();
});

router2.get('/test', async (ctx, next) => {
    console.log('match /test');
    ctx.body = 'match /test';
});

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

app.listen(3000);

Visit localhost:3000/test, will get a 404 response. And the logs:

/test
match anything

2 Course

After some work of debug, I found the problem:

In dispatch() method (middleware), It will set layer.path to ctx.routerPath, for the example above, it's /.*/ (RegExp), Then, the second router get the value, path becomes /.*/, but it should be /test which the request carries with.

Router.prototype.routes = Router.prototype.middleware = function () {
  const router = this;

  let dispatch = function dispatch(ctx, next) {
    debug('%s %s', ctx.method, ctx.path);

    // THIS LINE!!
    // the `path` may get value from `ctx.routerPath`
    const path = router.opts.routerPath || ctx.routerPath || ctx.path;

    const matched = router.match(path, ctx.method);
    let layerChain;

    if (ctx.matched) {
      ctx.matched.push.apply(ctx.matched, matched.path);
    } else {
      ctx.matched = matched.path;
    }

    ctx.router = router;

    if (!matched.route) return next();

    const matchedLayers = matched.pathAndMethod
    const mostSpecificLayer = matchedLayers[matchedLayers.length - 1]
    ctx._matchedRoute = mostSpecificLayer.path;
    if (mostSpecificLayer.name) {
      ctx._matchedRouteName = mostSpecificLayer.name;
    }

    layerChain = matchedLayers.reduce(function(memo, layer) {
      memo.push(function(ctx, next) {
        ctx.captures = layer.captures(path, ctx.captures);
        ctx.params = ctx.request.params = layer.params(path, ctx.captures, ctx.params);

        // THIS LINE!!
        // put the value `layer.path` to `ctx.routerPath`
        ctx.routerPath = layer.path;

        ctx.routerName = layer.name;
        ctx._matchedRoute = layer.path;
        if (layer.name) {
          ctx._matchedRouteName = layer.name;
        }
        return next();
      });
      return memo.concat(layer.stack);
    }, []);

    return compose(layerChain)(ctx, next);
  };

  dispatch.router = this;

  return dispatch;
};

3 More

This piece of code comes from #93 , which tries to fix #34 . I invested the issue, I think maybe the behavior before the fix has no problem, it should be the intended action. Maybe this needs more discussion.

The last thing, this PR breaks a test case, which tests ctx.routerPath should exists.

koajs / router