-
Notifications
You must be signed in to change notification settings - Fork 24
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
Preserving query params across route changes #13
Comments
@tscizzle what version of React Router do you use? |
i've just tested your issue on v3 (router v3 = history v3): By the way, why do you use direct history.push and not router.push? |
I have not sufficiently tried combining this module with a router, but my understanding is that things should work as long as the router changes only the path; you may have to make yourself a small helper function to ensure it does not change anything else. (like @dmitkataev's example, I would read from
Or, using object-rest-spread syntax:
I do not understand how this problem could happen. If the update of the path is atomic (no pauses between reading and setting location), that dispatched action's effect on the query params should either happen before or after the update of the path, and things should go just fine. If you are able to create a simple example that proofs me wrong, please share it. It could well be that I have made wrong assumptions about the implementation of |
@dmitkataev I'm using v4 @Treora Consider a component that has import { withRouter } from 'react-router-dom';
let MyComponent = ({ someVal, setStateToSomeVal, location, history }) => {
const handleClick = () => {
setStateToSomeVal(someVal);
history.push({
pathname: '/nextPathname',
search: location.search, // location.search is A, `setStateToSomeVal` setting the query param to B is not reflected here
});
};
return <button onClick={handleClick}> Click me </button>
}
MyComponent = withRouter(MyComponent) After clicked, If, in the click handler, after (Note that it yields the same situation when using |
@dmitkataev Also, using direct What is this |
@tscizzle you can get an access to a router object via context, defining a static variable in your component class, f.e. |
Partially related to this issue is the fact that the hash will also get squashed.
|
@Pringels you are right! |
@tscizzle Thanks for the code example. I understand how it goes wrong when reading the
If they are different, could the problem possibly be that ReactQuerySync is using a different |
Ok, I passed ReactQuerySync my To summarize, for anyone following along, or coming across this via search: I was using react-router and was not explicitly using the history library. I used react-router's import { BrowserRouter } 'react-router-dom'
...
<BrowserRouter>
...
</BrowserRouter> @Treora asked if I passed ReactQuerySync the So I made the following changes:
Example of each of those things: // src/myHistory.js
import createHistory from 'history/createBrowserHistory';
const history = createHistory();
export default history;
// whichever component your router lives
import history from './myHistory';
...
<Router history={history}>
...
</Router>
// src/myReduxQuerySync.js
import history from './myHistory';
...
const enhancer = ReduxQuerySync.enhancer({
params: {
...
},
initialTruth: 'location',
history,
});
... |
@tscizzle thanks for sharing! It is unfortunate that this needs such tricks to work. It would have been much easier if the I guess the best we can do is just provide instructions for how to set this up correctly. |
Added a small hint to the readme with a link to your explanation (commit 96784cb). Closing this issue. |
React Router's
history.push('/thing')
squashes the query params (what it callslocation.search
).One can do
history.push({pathname: '/thing', search: location.search})
, to preserve thelocation.search
, but an action could be dispatched just beforehistory.push
which changes the query params, so thelocation.search
thathistory.push
is using is now out of date, and it again squashes the query params.It's making it really tough for me to use this package, which is otherwise dope.
Any ideas?
Nice package, btw!
The text was updated successfully, but these errors were encountered: