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

Coerce query params to string - alternate solution #4106

Conversation

jochenberger
Copy link
Contributor

@jochenberger jochenberger commented Oct 27, 2016

See #3863:
On one of my pages, where I have a paging table, I store the page number as a query parameter. I use a number in the Link target, e.g. Page 2.
When the component re-renders after I click on the Link, props.location.query.page is a number. However, if I reload the page so that window.location.search is parsed again, props.location.query.page is a string.
Please review #4107 before this.
See also #4099.

@jochenberger jochenberger changed the base branch from master to v4 October 27, 2016 07:03
query: { a: undefined, b: null }
}, {
pathname: '',
query: { a: undefined, b: null },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it even let undefined through here? If we parse ?b, the query won't have an a key.

@mjackson
Copy link
Member

I'm afraid this PR is out of date. We're totally out of the query parsing/stringifying game in v4. All we give you is the string version, location.search. Parsing and serializing are in userland now.

@mjackson mjackson closed this Jan 11, 2017
@nikitatrifan
Copy link

@mjackson could you explain, please, why you're "totally out of the query parsing/stringifying game in v4"?

@mjackson
Copy link
Member

mjackson commented Jan 12, 2017

@expdevelop A few reasons:

  • Letting users do their own query parsing/stringifying gives them more flexibility. They can choose to use node's querystring lib, Browserify's querystring shim in the browser, qs, query-string or anything else they prefer. There are quite a few options, and they each have different strengths.
  • It's a lot easier and faster to compare location.search === otherLocation.search than deepEqual(location.search, otherLocation.search). This benefit plays out in lots of places both inside the router and in userland code.
  • Not shipping a query lib with the router lets us be significantly more lightweight.

Also, it's an easy switch for users!

<Link to={{ query }}>
// becomes
<Link to={{ search: stringify(query) }}>

If you find yourself doing this a lot, you'll probably wanna just wrap <Link>:

import stringify from 'my-fav-qs-lib'

const LinkWithQuery = ({ to, ...props }) => (
  <Link {...props} to={{ ...to, search: stringify(to.query) }}/>
)

<LinkWithQuery to={{ query }}>

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants