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

Merge query string into params #214

Closed

Conversation

bringking
Copy link

@bringking bringking commented Jul 26, 2018

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
Copy link

Coverage Status

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

@bringking
Copy link
Author

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

@samosaboy
Copy link

Seconded, this is required for us too.

@bringking
Copy link
Author

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

@bringking
Copy link
Author

@fridays bump

@elisechant
Copy link

Wouldn't you write this in to pattern instead?

@bringking
Copy link
Author

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

@javiercr
Copy link

@bringking you probably wanna ping @elliottsj here, see: #244

@ruaridhnewman
Copy link

@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!

@bringking bringking closed this Apr 8, 2020
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.

6 participants