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

V6.7.0 breaks the location state flow #394

Closed
alamchrstn opened this issue Feb 17, 2020 · 5 comments · Fixed by #399
Closed

V6.7.0 breaks the location state flow #394

alamchrstn opened this issue Feb 17, 2020 · 5 comments · Fixed by #399

Comments

@alamchrstn
Copy link

Hi, I tried to update to v6.7.0 and I found out that it breaks the flow of the app (maybe most of the apps that use this will get this issue also). It's caused by the inclusion of location's state into the payload of the redux action, thus it's causing the app to be re-rendered multiple times and also resulting in pushing too many stacks into the browser history.

history.push({
pathname: '/other-page',
state: responseData,
});

if state is not passed, then it works as normal. Maybe someone could come up with a solution for this? In the meantime, I revert back to version prior to v6.7.0

Thanks!

@richardchen85
Copy link

I have the same problem. After i looked into the source code, I found that the state object stored in store is different from the one stored in history.

So, if you want to continue to use v6.7.0, you can use JSON.stringify(state) to serialize the state into string:

history.push({
  pathname: '/other-page',
  state: JSON.stringify(responseData),
});

@BlazPocrnja
Copy link
Contributor

Hello everyone first time contributor long time programmer.

if (
props.history.action === 'PUSH' &&
(pathnameInHistory !== pathnameInStore ||
searchInHistory !== searchInStore ||
hashInHistory !== hashInStore ||
stateInStore !== stateInHistory)
) {
this.inTimeTravelling = true
// Update history's location to match store's location
history.push({
pathname: pathnameInStore,
search: searchInStore,
hash: hashInStore,
state: stateInStore,
})
}
})

On line 48, this comparison doesn't work if the location's state is an object, meaning it returns true every-time and results in multiple locations being pushed to history.

I don't think serializing the state into a string is the best long term solution. I would prefer to use my state as an object as it has been in previous releases. If there's a reason not to please let me know!

Anyways just doing a comparison between objects here seems to fix the problem for me 100%, so I'd like to maybe make that change to this repo?

BlazPocrnja added a commit to BlazPocrnja/connected-react-router that referenced this issue Mar 9, 2020
@alamchrstn
Copy link
Author

I would definitely agree with @BlazPocrnja , comparing the location's state would need to consider object comparison.

@EDcjukna
Copy link

I agree with @BlazPocrnja but I think it might also be valuable to be able to supply your own function to determine the equality of objects.

This could be done with lodash isEqualWith, this could be an optional function prop passed to the ConnectedRouter.

@BlazPocrnja
Copy link
Contributor

@EDcjukna I like that idea actually! I'll put something together in my pull request. #399

BlazPocrnja added a commit to BlazPocrnja/connected-react-router that referenced this issue Mar 16, 2020
BlazPocrnja added a commit to BlazPocrnja/connected-react-router that referenced this issue Mar 21, 2020
BlazPocrnja added a commit to BlazPocrnja/connected-react-router that referenced this issue Mar 21, 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 a pull request may close this issue.

4 participants