-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
No more parsing of query strings in V4? #4410
Comments
I have just discovered the same thing, and was rather surprised. I understand that the I've seen it mentioned that history could be modified to add query parsing support back in, but without any suggestion how. Seems like an example be useful here, at the very least. Particularly since this is a regression in v4 compared to earlier versions. |
There are a number of popular packages that do query string parsing/stringifying slightly differently, and each of these differences might be the "correct" way for some users and "incorrect" for others. If React Router picked the "right" one, it would only be right for some people. Then, it would need to add a way for other users to substitute in their preferred query parsing package. There is no internal use of the I don't know of a great way to add automatic query parsing to the history instance, but I think that you could listen for location changes and modify the Having included that, I do think that it would just make more sense to just parse Edit I removed the sample code since people are pointing out that it had issues. It was just something that I wrote in a minute or two without actually testing, so I don't want more people to keep running into problems because of it. The |
Solid answer, thank you @pshrmn :) |
Sorry, but I just do not follow the logic here. Path params and query string params are two equally valid and popular ways of supplying data through the URL. If the purpose of the library is to support syncing the URL to the UI, then why is parsing and binding one type of param to the match object considered in scope, and the other not? Why only solve half the problem? Nor do I see any reason why a user would need to supply a particular query string parsing library, any more than they would need to override the router's current path parsing algorithm. They would simply need to use the query string in the way that the router expected, just as they currently have to use the path in the way the router expects. |
@jonrimmer We've had many, many requests over the years to modify the way we handle query string parsing and serialization. Just had another one (for v3) this morning, in fact! Just because you don't see it, doesn't mean people don't have different needs. Also, it's incredibly easy for you to provide your own solution here. import stringifyQuery from 'whatever-lib-you-want'
<Link to={{ pathname: '/the/path', search: stringifyQuery({ your: 'query' }) }}>click me</Link> Still too much work? const QueryLink = (props) => (
<Link {...props} to={{ ...props.to, search: stringifyQuery(props.to.query) }}/>
) Now you can <QueryLink to={{ pathname: '/the/path', query: { go: 'nuts' } }}>click me</QueryLink> |
One thing that's become cumbersome in v4 is creating links with some query parameters updated. With v3, I used to do this.props.push({
pathname: this.props.location.pathname,
query: Object.assign({}, this.props.location.query, { foo: "bar" })
}); That means that all other potential query parameters stay in place. |
@jochenberger It's literally just this.props.push({
pathname: this.props.location.pathname,
search: stringifyQuery(Object.assign({}, parseQueryString(this.props.location.search), { foo: "bar" }))
}); And if that's still too much work, you can wrap it up in a component as I demonstrated in #4410 (comment) |
Thanks, I ended up writing a helper function doing about that. Of course, doing it like this means that you have to parse |
I don't mean to beat a dead horse, but this is kind of a bummer 😕 I'd prefer an opinionated solution over no built in solution. The way v3 handled it was good. Ah well, for now looks like the query-string library mentioned above is all I really need. Thanks all for the hard work on v4! It would be good to document this behavior for future adopters, query strings being as popular a feature as they are—had to dig a bit to find this thread. |
@rubencodes The approach being taken for 4.0 is to strip out all the "batteries included" kind of features and get back to just basic routing. If you need query string parsing or async loading or Redux integration or something else very specific, then you can add that in with a library specifically for your use case. Less cruft is packed in that you don't need and you can customize things to your specific preferences and needs. |
This is a very important topic. Most routing libs handle query params. I fully understand the reasons against it in v4 and totally support it. For those coming from those other libs, and especially v3 - it might be worth it to mention this whole topic (or at least #4410 (comment) and #4410 (comment) )in the docs though. I searched for this for two days (I am horrible at searching, first place I looked was docs) This comment here in this topic was also very very helpful to me to understanding v4 methodology - #4527 (comment) The other thing I struggled with was picking the query string lib. I found this great topic that told the difference between |
FYI: For a lot of cases
We use the https://github.com/jerrybendy/url-search-params-polyfill for older browsers which works great with webpack. |
Ok for not including query string parsing inside React Router but since this is a very common use case, it should be documented here. |
I'm really missing the query, and I think many other users expect this to be a core functionality in a router. Currently I see only two suboptimal possibilities, either like @donaldpipowitch, getting the query via URLSearchParams and setting link search by stringifying query on each of your Links (pretty inefficient!) or overriding router components (e.g. Link, withRouter), something like: export const Link = (props) => {
if (props.to && props.to.query) {
props.to.search = stringifyQuery(props.to.query);
delete props.to.query;
}
return <LinkLegacy {...props} />;
};
export const NavLink = (props) => {
if (props.to && props.to.query) {
props.to.search = stringifyQuery(props.to.query);
delete props.to.query;
}
return <NavLinkLegacy {...props} />;
};
export const withRouter = (WrappedComponent) => {
@withRouterLegacy
class WithRouter extends Component {
static contextTypes = {
router: PropTypes.shape({
history: PropTypes.shape({
push: PropTypes.func.isRequired,
replace: PropTypes.func.isRequired,
createHref: PropTypes.func.isRequired
}).isRequired
}).isRequired,
};
push = (propsTo) => {
const to = { ...propsTo };
if (to.query) {
to.search = stringifyQuery(to.query);
delete to.query;
}
this.context.router.history.push(to);
}
replace = (propsTo) => {
const to = { ...propsTo };
if (to.query) {
to.search = stringifyQuery(to.query);
delete to.query;
}
this.context.router.history.replace(to);
}
render() {
const { location } = this.props;
location.query = parseQuery(location.search);
return (
<WrappedComponent {...this.props} router={{ ...this.context.router, push: this.push, replace: this.replace }} />
);
}
}
return WithRouter;
}; I don't find either satisfying. Why not give a possibility to define parseQuery/stringifyQuery on the wrapper components (BrowserRouter, StaticRouter, ..) and use these instead of stripping query completely @timdorr ? |
And what about having another package like |
Ty Donald for that post. Adding a late comment for anyone else searching this post for how to update their code, and they aren't sure what package to grab, 'query-string' is simple one, and worked for me after searching some recommendations on stack overflow. I love React, and so far love all the v4 changes. in this however, complete opinion: The team made an overthinking move to get rid of this awesome and simple feature. I love that v4's goal is to simplify, so this thinking does the opposite, just to satisfy the loud minority. Sends the signal that if we send them enough emails they will change things to how we want them. I fully understand it, but I'd say the quiet majority of us who haven't been emailing are against any 'upgrades' that make us add lines to our code. Despite that, I still appreciate all the hard work the team does! I feel I have no room to complain because the team made it in the first place :) |
@pshrmn const history = createBrowserHistory()
// register the first listener. These are called synchronously, so
// the next listener won't be called until this has finished
history.listen(() => {
history.location = Object.assign(history.location,
// parse the search string using your package of choice
{ query: parseQueryString(history.location.search) }
)
}) when a user refresh the browser. history.listen doesn't trigger so the location.query === undefined .how to get location.query when get the page at the first visit . |
You can use this to decode query String
|
@lcoder Add the query outside the listener.
|
@v1kku Thanks! |
I personally believe that it is a poor decision to leave this out. Now everyone has to bake there own solution rather than the picky few where the default doesn't work / fit, they (picky few) would have previously been the only ones who would need to bake their own solution. Now I've been following react router v4 for a while now (and the project from v1) and it's taken so many huge steps which in some cases have been reversed. One of the most difficult projects from an early stage that I've ever followed. I understand it's not stable etc but how are people supposed to contribute in such a situation. I respect the work of people here, I just find it very frustrating that changes are made without any justification or even documentation / notes that parts have changed / been removed. I find myself trawling issues to find that parts have been removed or are deprecated!!! |
Could not agree more. Removing this feature without giving an alternative, or even documenting it is a huge oversight. Also, the "correct" way of doing it could be solved by allowing users to pick their library. Similar to how mongoose selects their promise library. |
I would assume parsing querystrings is a fairly common task, and it would be great if the docs said something about best practices for using/including a parsing library 👍 |
"you should still just stringify the object yourself" ...or find a routing library that's willing to, you know, route. I'm trying real hard not to be dick, but this is madness. I don't even know what to say at this point. |
@odigity It does route. Query string parsing is another matter entirely. Again, the are multiple ways to handle it (qs, query_string, and querystring all are different for arrays, for example) and we don't have an opinion about which is "best". But most of all, this isn't the router's job. It's the underlying history library that caused the change. If you want to effect change, start there. |
Maybe some of the tension could be alleviated if a good set of examples could be put together that shows how all the native features in v3 could be accomplished in v4. Given that in v3 there was "the way" to do something, it seems logical that a set of examples would at least demonstrate "a way" to do the same thing that would handle 95% of use cases. If those examples didn't cover a particular use case then, as has been mentioned numerous times, we could always "roll our own" solution. But most people would be OK with an opinionated way especially if it is basically the way things were handled in v3. Without meaning to cast any aspersion at all on those who have graciously spent their time working on these modules, I do think it would have been nice if there could have been a react-router-core module acting like react-router v4 does now along with a more full featured react-router (react-router-core + opinionated addons) to ease the transition from v3. |
The As a result of it dropping query parsing, we have to drop it as well. Further complicating matters, our chosen library for pathname,
So, it's a combination of a few factors that ended on the decision of dropping query string parsing:
I am acutely aware that this is a thing everyone wants, but it has some severe drawbacks and creates a far more opinionated library. We're trying to be simple and generic. That's one of the central theses of 4.0. If people want this resolved (hell, I want this resolved!), then there are two bits that need to happen:
I know that puts the ball in someone else's court. But that's what open source is all about. If you want to see something changed, don't just pitch a fit, actually write some damn code. We're very open to expanding the packages hosted in this repo, now that we have the monorepo format established. That's why we brought in react-router-redux and react-router-config. We want more! And this is exactly the kind of thing that we want to support. Please help us! |
Matching query strings is complicated and rarely needed, which is probably why the utilities you depend on don't support it. So don't do that. Just parse the query string portion into an object and make it available in the component, and add support for serializing an object to a query string in the Link component. That's all most people need, and both are relatively straightforward. If you're concerned about committing to a query string parsing library, and don't want to pick a solution for corner cases (like arrays), and don't want to add dependencies / increase total size... don't. Just give us two hooks for providing our own serialize/deserialize methods and let us supply them once when starting our app. (You can even count on us for creating examples for the popular parser libraries to add to the docs or wiki.) You know, similar to @loCoder's example above, except actually working, which that didn't. |
If you want just query string parsing and serialization, that's a history concern. Definitely follow and chime in on remix-run/history#478 |
Thanks @timdorr, will do. |
In our case mostly do two things:
We did not want to use
Get the path, search and query
and inject
Coming from ReactRouterV2 we also try to dodge the next API change ;) and tried to decouple the router context from our components. |
I understand the 'there is no right way argument' but I think there should at least be a note about this in the documentation. I spent too long scratching my head over this and it wasn't until I finally found this issue thread was I able to figure it out. Something as simple as: |
All this looks very strange for me, because reading query string is only a part of routing. Right now there will be giant amount of boilerplate code that replaces params in query string with new values and I don't see any documentation or hints for history: How to push new history when I change query string from code? |
I've seen some people here saying that query string is not part of routing and that the reason it was removed is that it's not part of the standard (who cares that we've been using them for like 20+ years). I completely disagree with that. In a normal application a page on I can live with query strings not being parsed by using There, even though I know this is not Stack Overflow, I'd like to ask a question seeing how there's no documentation about this problem whatsoever - is there a way to force component to rerender on a query string change? I have pagination |
@codeaid When the The below code should illustrate the idea: // <Route path='/users' component={Users} />
class Users extends React.Component {
componentWillMount() {
this.fetchUsers(this.props.location.search);
}
componentWillUpdate(nextProps) {
this.fetchUsers(nextProps.location.search);
}
fetchUsers = (search) => {
const { page = 1 } = parse(search);
UsersAPI.get(page).then(data => {
this.setState({ users: data.users });
});
}
render() {
return (...);
}
} |
@pshrmn Thanks for the tip! I somehow didn't realise that the router-aware components will get new properties pushed into them! Ended up implementing a HOC which I can now use to inject import * as React from 'react';
import {RouteComponentProps, withRouter} from 'react-router';
import {ISearchQueryComponentProps} from 'custom-types';
import {SearchQuery} from 'utils/SearchQuery';
export function withSearchQuery<P>(
Component: React.SFC<P & ISearchQueryComponentProps> | React.ComponentClass<P & ISearchQueryComponentProps>
): React.ComponentClass<P & ISearchQueryComponentProps> {
// higher order component class
class WithSearchQuery extends React.Component<P & ISearchQueryComponentProps & RouteComponentProps<any>, {}> {
/**
* Render current component
*
* @return {JSX.Element}
*/
render(): JSX.Element {
// ensure router properties don't get passed down the hierarchy
const {location, match, history, ...rest} = this.props as RouteComponentProps<any>;
const query = new SearchQuery(location.search);
return (
<Component
{...rest}
searchQuery={query}
/>
);
}
}
return withRouter(WithSearchQuery);
} Here's the To use it declare your component like this: class MyCustomComponent extends React.Component<IMyCustomComponentProps, {}> {
/**
* Render current component
*
* @return {JSX.Element}
*/
render() {
const {searchQuery} = this.props;
return (
<pre>
{JSON.stringify(searchQuery.getParams(), null, 2)}
</pre>
);
}
}
export default withSearchQuery<IMyCustomComponentProps>(MyCustomComponent); |
react-router 4 guys you are missing the point. The support for qs in V3 was inadequate to begin with. You only provided parsing of the values, but no real support. What you should have done in 4 is to make query string values a first class citizen in the router lib and allow routing based on them (just like params and url parts). There is no reason to distinguish between values embedded in the path part and the query string. All your answers regarding the ability to use 3rd party library for parsing the values, are only usable if you stay with the kind of support you offered in v3. For real query string support, they are not relevant. Alos, you argue that you removed support due to multiple contradicting requirements for query serialization and deserialization. But this can be easily solved by exposing 2 functions properties where the user can specify his own strategy for serialization if she does not like yours. Exposing the 2 function would have taken you less time than to read and answer all these angry people here, who have real needs from the field. And would have saved many hundreds of hours for the same people who see that all major routing library do what they need without over talking about it, and thus expect it. |
same issue here, don't see the query object containing query string parameters |
why do people speak here about only parsing query string: it only a part of the problem. For example, we have a page where we filter objects by some query string. Clicking "search" button must change query string, replacing the old variable with a new one. A resource on a web page is defined by path AND query string (and not anchor), so query string must be a part of routing mechanism. Putting query parameters to path usually leads to a very unmaintainable path and to a situation when there are many paths where filter segments take different places. I have added query string parsing and handling to current version of react router, but previous versions were much more convenient and I had to write a lot of useless boilerplate code now. I suppose that this question should be discussed more if you do not want to have pointless forks. |
I don't have a strong opinion about rather query strings should be handled by react-router or not. I only see two reasons why react router v4 dropped query strings parsing:
|
I get that its a fast paced and liquid environment and that often its just a few contributors who keep a project on track, but imho 'good' change management practices are as important to long term viability (and popularity) of a project, as good coding practices. As far as I can tell React-router is by no means a frequent or serious source of this kind of thing but this is certainly an example of it. Although in this case, for me at least its not that big a deal (even if I happen to think dropping it is poor decision) - I just started using r-r so I'll go research another library but I pity poor Coder who has a serious investment in 3.X and is now cut off from the upgrade path without dropping 'n' other projects with waiting clients, to rework n^? others. Multiply that by the number of packages that do this to Coder and it's no wonder its fast paced and liquid environment. Just sayin.
|
i was able to deal with it but had to made changes such as start using
|
@mjackson The only problem with your code is that you can't easily tell what's the meaning behind It's like explaining someone to do |
|
that's how I dealed with it import { createHashHistory, History as BaseHistory, LocationState, Path } from 'history';
import * as H from 'history'
import { HashRouterProps } from 'react-router-dom'
import { parse, stringify } from 'qs';
export class Location implements H.LocationDescriptorObject {
private _location: H.Location
private _cachedQuery: any = {}
private _prevsearch?: string
constructor(location: H.Location) {
this._location = location
}
get query() {
if (this._prevsearch !== this._location.search) {
this._cachedQuery = parse(this._location.search, { ignoreQueryPrefix: true })
}
return this._cachedQuery
}
get pathname() {
return this._location.pathname
}
get search() {
return this._location.search
}
get state() {
return this._location.state
}
get hash() {
return this._location.hash
}
get key() {
return this._location.key
}
}
interface LocationDescriptorObject extends H.LocationDescriptorObject {
query?: { [key: string]: any }
}
export class History implements BaseHistory {
private _history: BaseHistory
constructor(props: HashRouterProps) {
this._history = createHashHistory(props)
}
get length() {
return this._history.length
}
get action() {
return this._history.action
}
get location() {
return new Location(this._history.location)
}
push(location: LocationDescriptorObject): void
push(path: H.Path, state?: H.LocationState): void
push(a: H.Path | LocationDescriptorObject, b?: H.LocationState): void {
if (typeof a === 'string') {
return this._history.push(a, b)
}
if (a.query !== undefined) {
a.search = stringify(a.query, { addQueryPrefix: true })
}
return this._history.push(a)
}
replace(location: LocationDescriptorObject): void
replace(path: H.Path, state?: H.LocationState): void
replace(a: H.Path | LocationDescriptorObject, b?: H.LocationState): void {
if (typeof a === 'string') {
return this._history.replace(a, b)
}
if (a.query !== undefined) {
a.search = stringify(a.query, { addQueryPrefix: true })
}
return this._history.replace(a)
}
go(n: number) {
return this._history.go(n)
}
goBack() {
return this._history.goBack()
}
goForward() {
return this._history.goForward()
}
block(prompt?: boolean) {
return this._history.block(prompt)
}
listen(listener: H.LocationListener) {
return this._history.listen(listener)
}
createHref(location: H.LocationDescriptorObject) {
return this._history.createHref(location)
}
} and then
|
There are many ways to get a // This is probably the most straightforward way. You'll get a `location`
// prop in your route component. Just parse `location.search` and away
// you go!
const HomePage = ({ location }) => {
const query = new URLSearchParams(location.search)
// When the URL is /the-path?some-key=a-value ...
const value = query.get('some-key')
console.log(value) // "a-value"
}
<Route ... component={HomePage}/> If you are targeting a browser that does not support the URL API, a good option is the query-string package. import qs from 'query-string'
const HomePage = ({ location }) => {
const query = qs.parse(location.search)
} Also, remember you get the route component's props in the <Route ... render={({ location }) => {
const query = qs.parse(location.search)
// ...
}}/> As mentioned earlier, more and more browsers are evolving to support URL parsing natively. In fact, only IE 11 doesn't natively support it as of this writing. So we opted to not include a query parsing library with React Router to keep things light and also give users more flexibility over URL parsing. Thanks for understanding! |
I just upgraded from V4 alpha to beta (hooray!), but I can't find the
location.query
object anymore, only alocation.search
string. Have I missed something, or do I have to do my own parsing now?The text was updated successfully, but these errors were encountered: