search

fridays / next-routes

Universal dynamic routes for Next.js
MIT License
2.47k stars 230 forks source link

Merge query string into params #214

Closed bringking closed 4 years ago

bringking commented 6 years ago

We are running into an issue adding query strings to Next routes. For example, when viewing a "reviews" page like so - /someurl/:slug/reviews we need to paginate with a query string like -

/someurl/my-slug/reviews?page=1

This works fine if you push a shallow route with Link however, on SSR reloads, only the named param segments are added to the props.url.query for the page. This fixes this by merging params and the query with named params taking precedence

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.4%) to 94.468% when pulling 862465e1684c6174007b6982f9dbd867ac8bd4b6 on GhostGroup:feature/merge-query-strings into 08e8bbeaa76c5ec194abea388a193bedac7f35fc on fridays:master.

bringking commented 6 years ago

@fridays any chance on getting this merged? Any way that we can help?

samosaboy commented 6 years ago

Seconded, this is required for us too.

bringking commented 5 years ago

@fridays friendly bump, would love to get these fixes merged. Any way we can help or do you need maintainers on this project?

bringking commented 5 years ago

@fridays bump

elisechant commented 5 years ago

Wouldn't you write this in to pattern instead?

bringking commented 5 years ago

@fridays are you maintaining this anymore? If not, please let us know so we can fork and maintain ourselves. Or add a contributor 🙏

javiercr commented 5 years ago

@bringking you probably wanna ping @elliottsj here, see: https://github.com/fridays/next-routes/issues/244

ruaridhnewman commented 4 years ago

@Timer or @elliottsj could this PR be merged and released, as it fixes an issue we are having currently. Nextjs dynamic routing doesnt cover all the cases we need for our site, so we are using next-routes until it does but we are seeing this bug happening at the moment and is an easy fix. Thank you!