venantius / accountant

ClojureScript navigation for single-page applications, made simple.
Eclipse Public License 1.0
250 stars 39 forks source link

Navigating back in history to a path with a hash fragment strips the fragment from Accountants path #23

Closed PEZ closed 7 years ago

PEZ commented 8 years ago

When I load a url, say, http://foo.bar/baz#gazonk from scratch or click on a link /baz#gazonk in my SPA then my :nav-handler gets passed a path argument including the fragment (#gazonk), but when I navigate to the page using the browser's back button the path argument is stripped of the fragment. Is this intentional by Accountant, a bug or something Accountant can't do anything about, or what?

PEZ commented 8 years ago

If it's a bug, I'll be happy to try fix it with a PR.

venantius commented 8 years ago

Sorry, can you write this up in state machine style? I want to make sure I understand what you're saying. It sounds like you're saying there's a flow like:

  1. Navigate to http://foo.bar/baz#gazonk
  2. Navigate to http://foo.bar/flop
  3. Hit back button.
  4. Now your browser is at http://foo.bar/baz rather than http://foot.bar/baz#gazonk

Is that correct?

PEZ commented 8 years ago

Almost correct. =)

4a. Now my browser's location bar shows http://foot.bar/baz#gazonk. 4b. Accountant feeds my nav-handler with http://foo.bar/baz.

venantius commented 8 years ago

Oh that is interesting. That's a bug. Would welcome a PR :)

PEZ commented 8 years ago

OK. I sent a PR making it:

4a. Now my browser's location bar shows http://foot.bar/baz#gazonk. 4b. Accountant feeds my nav-handler with http://foo.bar/baz#gazonk.

I did it by instanciating the Html5History object using a custom transformer for setting and retrieving the token (instead of that set-token! function). Seemed like the right thing to do.

However, I'm starting to think that the hash fragment shouldn't be part of the path, but rather sent as a second parameter to the nav-handler. Saving users of Accountant from having to split the string apart if they are interested in only the path, only the fragment or maybe in both. Since Accountant was born out of the idea that you wanted to save coders from all having to solve the same problem, it makes sense to do it that way, right?

But that could be fixed in a later release, even maintaining backward compatability, I think. (Since it would change the arity of the nav-handler.)

venantius commented 8 years ago

I'm not a fan of splitting path and fragment in nav-handler. I don't want users to be forced to do things like path splitting within their application in order to work with accountant - it may be that they have to do that for other reasons, but if, say, they're working with a framework that's just handing them URLs I don't want them to have to do extra work there.

PEZ commented 8 years ago

Makes sense.

venantius commented 7 years ago

I believe this was resolved by #48, closing.