Skip to content
This repository was archived by the owner on Oct 26, 2018. It is now read-only.

compatible with HashHistory? #12

Closed
box-turtle opened this issue Nov 9, 2015 · 15 comments
Closed

compatible with HashHistory? #12

box-turtle opened this issue Nov 9, 2015 · 15 comments

Comments

@box-turtle
Copy link

I see a strange behavior if I try to use this with HashHistory and call syncReduxAndRouter.

The queryKey param (http://rackt.org/history/stable/HashHistoryCaveats.html) seems to change every second or so, changing in the URL bar and I think it's reloading the route every second as well.

image

I get this warning in console:

Warning: You cannot PUSH the same path using hash history

It's possible this is something with my app, but I didn't see it when using redux-router.

I like this API much better! Just curious if anyone else has seen this behavior.

@kadmil
Copy link

kadmil commented Nov 12, 2015

I've caught this too.

@kadmil
Copy link

kadmil commented Nov 12, 2015

Looks like side-effect of putting location-with-hash back to history. That should be it: then you update history of createHashHistory it tries to save pushState to hash and trigger updates.

@kadmil
Copy link

kadmil commented Nov 12, 2015

image
We've forgot about key part

  • looks like hashHistory treats url in a quite different way than window.location

@jlongster
Copy link
Member

Thanks all, I'll try to look at this more today. Admittedly I don't understand the key part of HashHistory.

@kadmil
Copy link

kadmil commented Nov 12, 2015

It's something they put as query param to save previous state, and it's quite puzzling.

@zeroshine
Copy link

I have the same problem too.
I find it's the problem of the function locationToString
In createHashHistory routing path is /xxxx, not include /#
but in locationToString will return location.hash include /#
so it can't avoid history pushstate

@jlongster
Copy link
Member

There's a PR for this: #23. I'd like to find a prettier way to fix, but looks like we basically just need to do something different in locationToString if using hashHistory. Will look at it more soon.

@amccloud
Copy link

@jlongster Does that PR disregard all query params from being stored in state?

Looks like the key param _k never makes it into the store so every time the store changes the syncing thinks the location changed because the key param is present in window.location. We could strip the key param from window.location before comparing to what is in the store. The key param can be renamed from _k so this would likely need to be a customizable configuration to handle it correctly.

I wish they would just remove state from history 😞

@zeroshine
Copy link

@amccloud
In PR#23, I just write a simple regex in locationToString function to match the string without ?.
You can check its diff for more information
A simple fix it way is just copy syncReduxAndRouter to your source code
and use your customized locationToString

@jlongster
Copy link
Member

Hey all, I just started working on a basic example, and I tried hash history with it and it worked fine: #35

What other steps do I need to do to recreate this issue?

@jlongster
Copy link
Member

@kadmil @box-turtle I still can't fully reproduce this error: "Warning: You cannot PUSH the same path using hash history"

We do need to fix something here, but it should never push the same path. Because if history changes because a Link was clicked, it will fire history.listen, and will do something like if("/foo" !== "/#/bar?_k=blah") which will trigger a store update. However it updates the store with the location from history, which is actually only /bar. That update will trigger another check in the store listener, and will compare "/bar" !== "/#/bar?_k=blah" because it gets the location from window.location. That fires another pushState, but when that history listener runs it will compare "/bar" !== "/bar" and stop there.

So there is a problem: the updating process should stop in the store listener. We can't assume that window.location is the same as the location given from the history object when using hash history. But I can't reproduce exactly what you say, and I'm curious why not.

@jlongster
Copy link
Member

@ryanflorence @mjackson Sorry to pull you in like this, but do you know of a robust way to get the current URL from a history object? Right now when I need to compare it against the URL in the redux store, I use window.location but that doesn't work when using hash history. I'd like for a way to do history.getCurrentLocation, which last I looked is not exported (I'll check again).

@heydemo
Copy link

heydemo commented Nov 22, 2015

@jlongster You can recreate the same path warning if you set

createHashHistory({ queryKey: false });

locationToString is including the leading #/ in the path so it will appear 'new.'

locationToString - '/#/mypath';

routing.path - '/mypath'

@amccloud
Copy link

@heydemo checkout PR #40

@jlongster
Copy link
Member

This should be fixed with #40, which I just pushed out with 0.0.9

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants