Skip to content

Conversation

@TooBug
Copy link

@TooBug TooBug commented Jul 1, 2021

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.

@3imed-jaberi
Copy link
Member

Fixed through this PR (#160)

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

Successfully merging this pull request may close these issues.

The ctx. _matchedRoute is not the match route.

2 participants