Same URL different state not pushed (only replaced)

[Macroz]

Hi!

I have popup-like components (i.e. tabs, off-canvas). I don't want them to show in the URL or as hashes, as I consider them to be transient state with regards to linkable "friendly" URLs. So I tried to use history PUSH with the same URL, but with a state where the popup is open or not. The problem is that since the URL is the same, I always get a REPLACE and never a PUSH.

This seems to be a feature of PR #43 I also noticed that you have already started fixing the consequences of it, such as issue #166

What do you think about my case? Why should you not be able to push only state changes? Could you comment this? I also don't quite understand the comment in code "treat PUSH to current path like REPLACE to be consistent with browsers".

Thanks!

[taion]

Okay, to clarify, you're doing something like history.pushState(state, path), where you change state but not the path, and you're seeing that your PUSH gets changed to a REPLACE?

If that's the problem, that sounds like it could be reasonable for us to fix. @mjackson @taurose thoughts?

[Macroz]

Yes, correct. I created a sample PR #179 that fixes my problem by comparing states also.

[agundermann]

I'm not sure you always want it to behave like that though. How about adding something like shouldReplace(prevLocation, nextLocation): bool to opts instead?

[taion]

@taurose To my mind, PUSH-ing with state is like POSTing a form. It seems like it should result in a new history entry.

[Macroz]

For clarity, I use history.push() when I want to PUSH, and history.replace() when I want to REPLACE. Why does transitionTo(), which handles both cases internally, change a PUSH to same URL as REPLACE, in the first place?

[agundermann]

@Macroz It's because browsers don't create multiple history entries when clicking a link multiple times on a static website either. On top of that, we can't push the same URL twice for hash history unless they're decorated with a unique query key.

I agree it's a bit confusing though now that the public API uses the word push instead of the more general router.transitionTo.

@taion I'd say generally it's about passing any data to the next page that you don't want to be contained within the path/URL. Another example could be referral information.

The way you describe POST requests sounds like you'd need to do nextLocation.state != null instead of deepEquals(prev.state, next.state).

[Macroz]

@taurose Thanks for the clarification!

I feel like the browser behavior is sane for static sites, but now we're talking about a programmatic API to history. I don't see why the programmer can't decide here, which part of the state is in the bookmarkable URL and which part is in the state object. It's the age of the SPA, right? The programmer defines how that action is implemented with regard to the browser history.

Sounds like if the state is different, the query key should not be the same either?

[agundermann]

Sounds like if the state is different, the query key should not be the same either?

Yes, when using state there should always be a query key. But when not using state/query key (which looks like it may be the default soon), pushing the current URL will be a no-op. So the back button as well as links to the current page would behave differently depending on whether you use hash or browser history, which is something we should try to avoid imo.

I don't see why the programmer can't decide here, which part of the state is in the bookmarkable URL and which part is in the state object.

Yeah, I want you to be able to decide what's best for yourself, which is why I proposed a more flexible shouldReplace option.

I feel like the browser behavior is sane for static sites, but now we're talking about a programmatic API to history. [...] It's the age of the SPA, right?

I agree, but I think generally we should also consider the end users which are accustomed to the way static websites work and provide that behavior as a reasonable default. Same reason we're also trying so hard to provide bookmarkable URLs and support the browser back button.

I guess there's an argument to be made that that should be the responsibility of the router and not the history abstraction though. So I'd also be okay with reverting the PR and baking this into the router somehow. But then we'd have to find a solution for #163 that doesn't run into https://github.com/rackt/history/blob/master/modules/createHashHistory.js#L114 .

I just don't see much benefit in the approach of #179. I fear that by doing that we'd just be shifting the problem. E.g. to guarantee a PUSH you'd have to add a random id to the state; to get the current behavior, you'd now have to inspect the current location and then call replace or push manually.

[taion]

That we have it in transitionTo is neither here nor there, BTW, because transitionTo isn't (fully?) a public API.

@taurose If you care about the referrer, doesn't that imply that you want something like a PUSH rather than a REPLACE? It's tricky - I think we will need something like shouldReplace, but I think a better default would be to not change the PUSH to the REPLACE when state is in play.

[agundermann]

I was referring to the 0.x router.transitionTo(..).

If you care about the referrer, doesn't that imply that you want something like a PUSH rather than a REPLACE

Not necessarily I think. I'd tend towards pushing if the page looked vastly different like in the pinterest example. If the difference was minor or even unnoticable (eg. when using referral data for analytics) I'd prefer to use REPLACE.

Just to be clear, you'd prefer createPath(prev) === createPath(next) && next.state != null over #179?

[hannesj]

Yeah, I want you to be able to decide what's best for yourself, which is why I proposed a more flexible shouldReplace option.

Don't we have push and replace in the public API in order to differ what the user wants to do?

[taion]

@hannesj Are you suggesting that we move this upstream into <Link> in React Router? That might be a sensible way to go.

[hannesj]

@taion, that sounds a sensible approach to me. history, is(?) just a thin wrapper around the different history implementations and <Link> would be the component making sure that the navigation acts similar to traditional <a/> tags

[taion]

@taurose What do you think?

[agundermann]

I agree that the router appears to be a better place for it. I'm not sure about putting it into <Link> because then users can't easily get the behavior without it or switch between custom <button> and <Link> for navigation.

[taion]

Tricky there - I don't think we want to make router.push not just be history.push. Maybe just abstract out the link click handler into a utility function?

[agundermann]

Yeah, I don't like changing the router.push behavior either. Do you mean extracting just the logic for deciding push or replace? How would that look like?

Another idea would be to have something like a router.transitionTo alongside router.push and router.replace to make it clear that it's not necessarily a push (or replace) and that the router will take care of that decision, while still allowing people to force a push (or replace). This could also at some point be useful for using replace while another async transition is in process (alternative to using listenBefore).

[taion]

Pretty much just pull https://github.com/rackt/react-router/blob/v1.0.2/modules/Link.js#L42 into a function. For example, I do this right now in React-Router-Bootstrap: https://github.com/react-bootstrap/react-router-bootstrap/blob/v0.19.3/src/LinkContainer.js#L23.

[agundermann]

Well, I guess that could help rebuilding <Link> behavior, but I was more looking for a programmatic equivalent of the transition behavior, not tied to React or DOM events.

[taion]

Well, maybe something like you had above... { force: true } on the location descriptor or something.

[hannesj]

Bump, any update on this?

[taion]

No. If you want to move this forward, I'd submit a PR.

[mjackson]

I think #179 is on the right track. Let's follow up there.