Why is a new location created when navigating to same location?

[pshrmn]

This was brought up in remix-run/react-router#5039, but is more appropriate here.

Basically, the problem is that if you are on page /here and click a link to /here, you end up with essentially duplicate (just different keys) location objects. Then, clicking the back button will bring you back to /here. This is obviously intentional because the PushSamePath test sequence exists, but I'm not completely sure why.

However, that seems to differ from how browsers handle clicking on links to the same location. They seem to essentially do a replaceState when clicking a link that points to the same location (if you are on /here, click a link to /other, click the back button so that you are back on /here and click a link to /here, it does not wipe out the future location). Calling window.location.href with the same path also does not wipe out future locations, so I think that behavior is different for users who use the forceRefresh option.

I'm not sure what the ideal solution would be. I feel like it should be history that accommodates this.

For example, if it was left to other packages to adapt to this, react-router-dom's <Link> would have to be able to detect when its to is the same as the current location and call history.replace instead of history.push.

My initial instinct would be that push should detect when it is pushing the same location and call replaceState instead of pushState.

[mjackson]

The PushSamePath test sequence exists only because it's not actually possible to push the same path using hash history (setting window.location.href = window.location.href will be ignored, i.e. does not fire hashchange) so that test was just ensuring that you actually could push the same path with browser history. Anyway, that only explains where the test came from. I'm not sure what the correct behavior is. Seems like the kind of thing that wouldn't be defined very well across browsers.

[pshrmn]

From the WHATWG browser spec

If the navigation was initiated with a URL that equals the browsing context's active document's URL

1. Replace the current entry with a new entry representing the new resource and its Document object, related state, and the default scroll restoration mode of "auto".
2. Traverse the history to the new entry.

Where "equals" is defined here (basically, (a.pathname + a.search + a.hash) === (b.pathname + b.search + b.hash))

[pshrmn]

I haven't done an exhaustive search, but the only place that I have seen a different behavior has been on GitHub repo pages.

If you go to https://github.com/ReactTraining/history and click the history link (in the header), it will reload the same page, but with a new history entry. This isn't "real" navigation, because GitHub is intercepting the event and making an AJAX call, then manually changing the URL using the history API.

They call history.pushState when the request is made, and then history.replaceState once the data has been loaded (I think that is what's going on. The code is obfuscated so it is difficult to say with certainty what they are doing, but I think that I am right).

I would say that 1) This is different than how this package behaves because they are manually calling history.(pushState|replaceState), whereas people that use this package have the methods called for them.

While my initial thought would be to modify push, I do wonder if it would be better for history to implement a separate method that determines the method to call for you. That way, users could still force a new history entry for the same location.

history.navigate(to);

navigate = (location) {
  if (isSameLocation(location, history.location) {
    replace(location)
  } else {
    push(location)
  }
}

[pshrmn]

As @vladshcherbin noted here remix-run/react-router#5363 (comment), earlier versions of history would turn a PUSH into a REPLACE when the new location was the same as the old.

I'm not sure if the new API that I proposed in #472 is the approach that you want to take here @mjackson, but that behavior should definitely be brought back in some form.

[aghreed]

Came here because I also discovered that duplicates were being created in history.

I tend to agree with the API that @pshrmn suggested. Seems to give us the best of both worlds - fall into line with how developers seem to assume history works when the same location is PUSHed, but allow explicit overriding of that behavior if need be.

[turijs]

Just discovered this issue as well. Any updates?

Seems to me this isn't the sort of issue that should be left unresolved. It may seem like an "edge case", but I could easily imagine a frustrated user spam-clicking a nav link (maybe because they are waiting for ajax content to load and they think it is stuck). Then their back button "stops working" and they have no idea why.

@pshrmn 's suggestion seems solid.

[mjackson]

Let's just fix push. If it doesn't behave the same way the browsers do, it should.

[pshrmn]

I think that it is a matter of intent. If push is rewritten to replace when the new URI is the same as the current URI, then someone would never be able to push the same location. Why would they want to do that? I have no idea. Should they be able to? Maybe?

The benefit of having a method explicitly designed to mimic anchor behavior is that history could provide that behavior without limiting how people can push.

[vladshcherbin]

@pshrmn users are forced to have a broken push since RR 4 with all troubles it brought. Browsers don't give you any options and by default history should work the same way as browsers do.

It would be great to have push fixed first and then you can think about extending api.

[pshrmn]

At the end of the day, I'm not super invested one way or the other. I think that a new function would be more correct (I would argue that push isn't broken, it just doesn't emulate anchor behavior), but updating push should work 99.99% of the time.

[mjackson]

Something just occurred to me: maybe clicking on links is different than calling history.pushState?

So, the current behavior is to create a new location in history.push because that's what history.pushState does:

However, there may be some special behavior in browsers that checks the behavior of a click on a link and decides "hey, this person is already at that URL. do nothing".

[pshrmn]

Yes, a link click isn't always a PUSH, although I believe navigating to the same location should be treated as a REPLACE instead of a no-op.

I wish there was a History API method for this middle ground since everyone has to roll their own "is it push or replace" functionality.

[mjackson]

What I'm saying is that history.push and history.replace should always do exactly what they say they will do, but that clicking on a <Link> in React Router should probably check the current location first to determine if it's going to do a push or a replace. It seems like that's what browsers are doing, no?

[pshrmn]

Yeah, that's the behavior that the above quote from the WHATWG spec describes. That could either be done in React Router's <Link> by inspecting the current location to determine which method to call or in history by adding a new method that compares locations and determines whether to push or replace and having the <Link> call that.

My comment about the History API was just that I wish there was a history.actLikeALinkState to go along with pushState and replaceState so anyone building on top of it doesn't have to do this comparison themselves.

[guidobouman]

A reason why you would want that method in history: We integrate an external library that shows Modals (Gigya) into our own app history. So next to using <Link> components we're also interacting with the history API directly. And I'm not really planning on writing this diffing logic for every project I have to integrate with history. I'd much rather integrate it in history.

[mjackson]

OK, sure. We can have a history.doWhatALinkWouldDo method. Seems like something that should be in history any way for the sake of people building other libs on top.

I believe you've suggested using history.navigate in the past, right @pshrmn? That's not a bad name. I think it'd be great if we could include the word "link" somehow in the name to indicate that we're mimicking the behavior of HTML links. What about:

• history.link
• history.followLink
• ??

[pshrmn]

I like history.link. navigate is what I went with in Hickory (the history implementation I wrote for my router), but only for lack of a better verb.

As far as implementation goes, I had written one in #472. I'm not sure if you want to go with something like that (I had moved the push/replace code out of the push and replace functions so that navigate could re-use it), but if nothing else the tests should be good.

[mjackson]

Thanks, @pshrmn. I regret not looking at that PR sooner and having this discussion when you first proposed #472. I'll have some time to take a look this week and get this done.

[dabit1]

It is a critical issue. Right now there is no way to prevent accumulate same url into history stack. It makes no sense... I'm going to use this closed (I do not know why) pull request #558

[dabit1]

Finally I found a solution. Basically monkey patching....

props.history.listen(location => {
  lastLocation = location
})

// monkey patching to prevent pushing same url into history stack
const prevHistoryPush = props.history.push
props.history.push = (pathname, state = {}) => {
  if (lastLocation === null ||
      pathname !== lastLocation.pathname + lastLocation.search + lastLocation.hash ||
      JSON.stringify(state) !== JSON.stringify(lastLocation.state)
  ) {
    prevHistoryPush(pathname, state)
  }
}

[guidobouman]

@dabit1 @vladshcherbin The problem with the proposed fix is that the history package wants to stay as close to the browser native history API as possible. By moving the <a> logic into the push state they would break the compatibility with window.history. So, the whole thing is just not that easy.

There is a new path though; Create a new method that does the same as your monkeypatch, but under a new name. Then let the ReactHistory <Link/> component utilise that method. This way everyone would be happy, without breaking compatibility. This involves two small PR's that are easily built, but need someone to take the time. You're welcome to do so.

We're getting closer, the problem just needs an owner. (I might pick this up when I have the time.)

[dabit1]

Hi @guidobouman @vladshcherbin, finally I created that pull request:
#570

👍

[chpio]

workaround for the error: https://filipmolcik.com/react-router-warning-hash-history-cannot-push-the-same-path/

edit: slightly less fucked version: https://gist.github.com/chpio/a5d4f7d73d6643780db20db163561a67

[sky93]

Finally I found a solution. Basically monkey patching....

props.history.listen(location => {
  lastLocation = location
})

// monkey patching to prevent pushing same url into history stack
const prevHistoryPush = props.history.push
props.history.push = (pathname, state = {}) => {
  if (lastLocation === null ||
      pathname !== lastLocation.pathname + lastLocation.search + lastLocation.hash ||
      JSON.stringify(state) !== JSON.stringify(lastLocation.state)
  ) {
    prevHistoryPush(pathname, state)
  }
}

Where should I put the code?

[dabit1]

@sky93 in the main route. Check it out:

import React, { Component } from 'react'
import {
  BrowserRouter,
  Route
} from 'react-router-dom'
import ModalCheck from './modal-check'

class MainRouter extends Component {
  render () {
    let lastLocation = null

    return (
      <BrowserRouter>
        <Route component={props => {
          props.history.listen(location => {
            lastLocation = location
          })

          // monkey patching to prevent pushing same url into history stack
          const prevHistoryPush = props.history.push
          props.history.push = (pathname, state = {}) => {
            if (lastLocation === null ||
                pathname !== lastLocation.pathname + lastLocation.search + lastLocation.hash ||
                JSON.stringify(state) !== JSON.stringify(lastLocation.state)
            ) {
              prevHistoryPush(pathname, state)
            }
          }

          return <ModalCheck {...props} store={this.props.store} />
        }} />
      </BrowserRouter>
    )
  }
}

export default MainRouter

[bbenejc]

I just had the same issue, and tried your workaround... but also found a few issues with it, so I tweaked it a little bit:

import React, { Component } from 'react'
import {
  Router,
  Route
} from 'react-router-dom'
import createBrowserHistory from 'history/createBrowserHistory';
import ModalCheck from './modal-check';

const customHistory = createBrowserHistory();
const prevHistoryPush = customHistory.push;
let lastLocation = customHistory.location;

customHistory.listen(location => {
  lastLocation = location;
});
customHistory.push = (pathname, state = {}) => {
  if (
    lastLocation === null ||
    pathname !==
      lastLocation.pathname + lastLocation.search + lastLocation.hash ||
    JSON.stringify(state) !== JSON.stringify(lastLocation.state)
  ) {
    prevHistoryPush(pathname, state);
  }
};

class MainRouter extends Component {
  render () {
    return (
      <Router history={customHistory}>
        <ModalCheck {...props} store={this.props.store} />
      </Router>
    )
  }
}

export default MainRouter;

Basically - I am using Router instead of BrowserRouter, and passing it a customHistory prop - docs

[mjackson]

I believe the right place to fix this is in react-router. Instead of introducing extra methods (and semantics) in history, we should just replace instead of push when the location pathname + search are the same and a link is clicked. history doesn't even have the concept of a link click, so this isn't the right place for it.

We'll follow up in remix-run/react-router#5362

[guidobouman]

I get that you want to keep the API as close to the browser native history API as possible. But this way, all apps that want to programatically navigate like a link will have to rebuild this diffing logic all the time: #470 (comment)

Or do you plan to introduce a higher-level API in react-router?