koajs / router

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

`router.url` behaves unexpectedly for a named route that has no path parameters. #37

Closed panva closed 4 years ago

panva commented 5 years ago

router.url behaves unexpectedly for a named route that has no path parameters.

See da1ab1f for the failing test - CI https://travis-ci.org/koajs/router/builds/614446723

Following the docs

router.get('books', '/books', function (ctx) {
  ctx.status = 204;
});

var url = router.url('books',
  {},
  { query: 'page=3&limit=10' }
);
// actual "/books"
// expected // actual "/books?page=3&limit=10"

06052cb fixes the tests but i fear it may have unintended side effects, it'd be better if someone with knowledge of the library took a look at this.

The workaround for 8.0.2 is to pass it like so, omitting the second argument which shouldn't be optional according to the docs.

var url = router.url('books',
  { query: 'page=3&limit=10' }
);
// actual "/books?page=3&limit=10"

Alternatively, fix the docs. As-is i can't rely on @koa/router because if this gets fixed my integration would break.

niftylettuce commented 4 years ago

Can you fix the conflict in test/lib/router.js and I can have another look? @panva

panva commented 4 years ago

@niftylettuce done

niftylettuce commented 4 years ago

@panva thank you for your help here. v9.3.0 has been released to npm and GitHub

https://github.com/koajs/router/releases/tag/v9.3.0

🎉