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

Fix base tag hash history create href #578

Merged

Conversation

microbouji
Copy link
Contributor

This is related to #577. This changes createHashHistory's createHref to produce absolute URLs only when a <base> tag is present on the page. It works like this:

const loc = {
  pathname: "/the/path",
  search: "?the=query",
  hash: "#the-hash"
}

// no base tag present on page
history.createHref(loc)
// -> "#/the/path?the=query#the-hash"

// base tag present with non-empty href
history.createHref(loc)
// -> "<part of current url up to hash>#/the/path?the=query#the-hash"

The net effect of this will be to allow the browser to correctly mark links as visited.

This is because, when a base tag is present, the relative hash url will point relative to the base as far as the browser is concerned ( e.g. "/the/base/#/the/path?the=query#the-hash") while a history push to that location will do the right thing and update the href to "<part of current url up to hash>#/the/path?the=query#the-hash" ignoring the base.

So when history is used in HashRouter for example, clicking a link will update the location correctly but it won't get marked as visited from the browser. Producing absolute URLs from createHref when the base tag is present fixes this. Always creating absolute URLs from createHref would make the code and tests a bit simpler, but I thought only doing it when a base tag is present was less obtrusive. Let me know what you think.

@amuzalevskiy
Copy link

+1

@tennox
Copy link

tennox commented Jun 15, 2018

+1

This also solves an issue which I have when the <base> tag's href is set (in my case used by Typo3).
HashRouter is redirecting from /page.html to /#/ on page load (during the createHashHistory() function )

@jboolean
Copy link

+1

@mwanago
Copy link

mwanago commented Jul 25, 2019

Any plans on merging this?

@mjackson mjackson mentioned this pull request Sep 12, 2019
25 tasks
@mjackson mjackson merged commit 288fc42 into remix-run:master Sep 12, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 12, 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.

None yet

6 participants