Add at least partial support for query parameters

[lydell]

I know there's been a previous issue about query parameters: #2. However, that issue only talks about how to link to a page with query parameters.

The bigger issue is that adding query parameters to any URL (in the address bar of the browser, for example) causes no routes to match! All 404s.

This is because req.url contains not only the pathname, but also the query string.

https://github.com/gvergnaud/nextjs-dynamic-routes/blob/3947447f7cc62182322c4a0bce4221b52e8ed15d/src/Router.js#L63

The query string needs to be stripped out before trying to match routes. https://github.com/pillarjs/path-to-regexp#usage

Please note: The RegExp returned by path-to-regexp is intended for ordered data (e.g. pathnames, hostnames). It does not handle arbitrary data (e.g. query strings, URL fragments, JSON, etc).

Then, the query string needs to be parsed and be passed on to the page.

I monkey-patched the router in my routes.js to fix the above problems:

const _ = require("lodash");
const Router = require("nextjs-dynamic-routes");

const router = new Router();

// Workaround missing query string support in nextjs-dynamic-routes. Query
// parameters caused routes not to match (404) and weren't passed on.
const { getMatchingRoute } = router;
router.getMatchingRoute = urlString => {
  // https://stackoverflow.com/a/51525433/2010616
  const url = new URL(urlString, "https://host/");
  const urlParams = _.fromPairs(Array.from(url.searchParams));
  // `.getMatchingRoute` is bound so it can be called directly.
  const { page, params } = getMatchingRoute(url.pathname);
  return { page, params: Object.assign({}, urlParams, params) };
};

router.add({ name: "home", pattern: "/" });

I haven't found a nice way to link with query parameters, but I still think that fixing the above issues alone is a good thing. I was very surprised when I started getting 404s. After triple-checking my route patterns I finally figured out it was because of a query parameter...

[gvergnaud]

Thanks for your report! I'll have a look and keep you posted

[gvergnaud]

I just published a new version: 2.2.0. It should fix your issue, and I added the possibility to specify query params on your Link components. You will be able to add them like this:

<Link route="user" id="2" queryParams={{ anything: 'here' }}>

Please test it and tell me what you think

[lydell]

Thanks! It works well. I have two pieces of feedback:

1. I don't think the query parameters should take precedence over the path parameters. For the route /product/:id I expect the URL /product/saw?id=hammer to show me a saw, not a hammer. And maybe we should document that you cannot use the same name for a path parameter and a query parameter (the path parameter always wins). (No big deal, you can just rename one of them.)

   Details

   I also thought about passing both the path and the query value as an array, like `url.parse` handles multiple query parameters of the same name in Node.js: ``` > url.parse("?foo=1&foo=2", true).query { foo: [ '1', '2' ] } ``` But Next.js only provides the _last_ value in such cases, so I think it's more consistent to only pass the path parameter value.

2. Could you document:

   • The queryParams prop of <Link>.
   • That you can pass queryParams to the different router methods, such as router.pushRoute("search", { queryParams: { q: "cats" } }).

[gvergnaud]

Fixed the precedence issue of the params in v2.2.1, and added some documentation in the readme. Thanks for your feedback!

[lydell]

Awesome, thank you for the super fast fixes! :tada:

One minor thing – I couldn't find any tests for path parameters taking precedence over query parameters? Feels like a tiny detail like that could easily regress when refactoring.

[lydell]

Hmm, I just installed 2.2.1 and the query parameters still take precedence. Are you sure that your fix works?

[lydell]

@gvergnaud Does the precedence thing work for you?