Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jul 5, 2020
Merged

Conversation

panva
Copy link
Contributor

@panva panva commented Nov 20, 2019

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.

@panva panva changed the title router.url(name, params, { query }) doesnt apply query when the route has no parameters router.url behaves unexpectedly for a named route that has no path parameters. Nov 20, 2019
@niftylettuce
Copy link
Contributor

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

@panva
Copy link
Contributor Author

panva commented Jul 5, 2020

@niftylettuce done

@niftylettuce niftylettuce merged commit 1b06bd1 into koajs:master Jul 5, 2020
@niftylettuce
Copy link
Contributor

@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

🎉

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.

None yet

2 participants