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

don't strip the hash off of the window path #51

Merged
merged 5 commits into from
Sep 24, 2015

Conversation

jeffreywescott
Copy link
Contributor

Without this change, when using createHistory or createBrowserHistory, the window.location.hash gets stripped off of the location, never to be recovered, breaking in-page navigation when using react-router.

@Azaeres
Copy link

Azaeres commented Sep 18, 2015

👍

@ThisIsNotTea
Copy link

👍 This is important stuff.

@mjackson mjackson merged commit f55e9e6 into remix-run:master Sep 24, 2015
@jackmoore
Copy link

This isn't fixed in 1.11.0. Using history.pushState with a hash strips the hash. Using history.pushState with a hash and a query strips both.

history.pushState(null, '/home#top') redirects to /home

history.pushState(null, '/home#top', {page: 2}) also redirects to /home

@janpieterz
Copy link

@mjackson are you aware of this? I'm not a 100% sure but it seems like it indeed still doesn't work (using latest history combined with react-router).

@zdavis
Copy link

zdavis commented Oct 6, 2015

Yes, seems to not be fixed in 1.12.1. I believe the problem is here:

https://github.com/rackt/history/blob/master/modules/createBrowserHistory.js#L70

...Location is being rebuilt without the hash

I'll try to put together a pull request.

@mjackson
Copy link
Member

mjackson commented Oct 6, 2015

Thanks for bringing this to my attention, @jackmoore. I think @zdavis is right about where the problem is. I'll see if I can make a fix.

@mjackson
Copy link
Member

mjackson commented Oct 6, 2015

I should also note that in your second example, @jackmoore, you should be sure you're using the useQueries enhancer. That will give you a 3rd query arg to pushState.

@mjackson
Copy link
Member

mjackson commented Oct 6, 2015

@jackmoore @janpieterz @zdavis This is fixed in 7012f9b

@mjackson
Copy link
Member

mjackson commented Oct 6, 2015

Fix released in 1.12.2

@janpieterz
Copy link

Awesome, thanks a lot for the quick effort!

@jackmoore
Copy link

@mjackson Thanks, but when I combine the hash with query, it's putting the query in the hash. I'm using react-router's Link component if that matters, and history 1.12.2. Am I doing something wrong here?

<Link to='/home#top' query={{test: true}}>Test</Link>

Redirects to: /home#top?test=true

@zdavis
Copy link

zdavis commented Oct 6, 2015

Thanks so much!

@mjackson
Copy link
Member

mjackson commented Oct 6, 2015

@jackmoore That's a slightly separate issue. Let's follow up in remix-run/react-router#2176

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

Successfully merging this pull request may close these issues.

7 participants