Link method which prevent same locations in the history

[dabit1]

I created a method that call push when current location and next location is different and call replace when they are the same.

It fixes a known problem which was reported for many users.
#470
#558
#507

[guidobouman]

Also, there's a createLocation method, why are you not using that one?

[guidobouman]

I think it looks good, but I have some questions that might improve the overall PR a little.

[kujukuju]

Is there a workaround for this issue? It seems like this has been an issue for the last ~3 years.

[dabit1]

@TrentWDB I did not have much time lately. In max 1week I hope to review it ;)

[guidobouman]

Hey @mjackson, I think @dabit1 has a great PR here.

It fixes the long standing issue with links & history.push in a solid way. Which would in turn allow for a small PR in react-router to finally let the Link component start behaving like a true link. 🎉

[nksfrank]

What's the status on this PR?

[guidobouman]

Waiting for the repo owner @mjackson to merge this, after that a new PR for react-router can be made.

[sabbiu]

I am also looking for this fix. What is the status?

[darklightblue]

@mjackson This type of fix seems to have been discussed ad nauseam for an incredibly long time; even a couple of PRs have vanished. Would be great to get this fixed, finally.

[quisido]

Hoping to bring attention to this. I'm also awaiting this fix.

[alonquil]

me too!!!

[mjackson]

Thanks for submitting this PR!

[mix3d]

@dabit1 any updates? Should someone else fix the suggested changes and recommit?

[undrafted]

Please can we have this PR updated @dabit1

[infalse]

Hello. Struggled with the same problem of react-router and react-router-dom Link's being PUSHing history even if i was on the same /route already. What worked for me was:

import { withRouter } from 'react-router' 
import { NavLink } from 'react-router-dom' 
import { default as BSNavLink } from 'reactstrap/lib/NavLink' 

export const StyledLink = styled(BSNavLink)...

const NavLinkLogged = ({ to, history, staticContext, ...rest }) => {
  const shouldUseReplace = history.location.pathname === to
  return (
    <StyledLink to={to} tag={NavLink} replace={shouldUseReplace} {...rest} />
  )
}

export default withRouter(NavLinkLogged)

replace attribute becomes true when current location matches to path. So first time you click will be PUSH, and the others REPLACE.

Hope is helps

[undrafted]

I solved this problem by checking 'match' instead and rendering an empty a tag vs NavLink accordingly:

{match && match.isExact
            ? <a className={clsNavLink}>{content}</a>
            : <NavLink to={url} className={clsNavLink}>
              {content}
            </NavLink>}

[rowrowrowrow]

Hey, just wanted to say that this fix is important for me. I run into this issue when trying to render a component via a route when that component has a dynamically generated state. Thanks!!!

[dabit1]

Hi @mjackson! I did some changes in order to have it ready to be merged. A lot of people is requesting it so if you agree I think it's a good improvment.

[mjackson]

Hey @dabit1, sorry for the delay here. Are you using React Router? If so, would you be open to fixing it there instead of here?

This library is designed to be a minimal abstraction over the browser's built-in history API. Someday we hopefully won't need it anymore. That's the only reason I'm hesitant to add more API that the browser has no equivalent for.

Having said that, I'm ready to move on this now. This bug is super annoying and has been here way too long. So whether we fix it here or in the router, it'll be fixed in our next release.

[dabit1]

Hi @mjackson ! Nice to meet you finally hehe. So yes, I'm using React Router. But the thing is that in some cases I prefer or I have to use the method push of history instead of the Link component from React Router. For example, if you follow the Container pattern in React every callback of every presentational component is controlled by a container so presentational components can't decide what they can do after clicking them. For that reason I need to use the method push from the container and this is the reason Link can't be used.

Anyway I understand your point since this package is not only to be used for React. Maybe makes more sense if react-router-dom has a method link apart of Link component since only with Link component we don't do everything we need.

[guidobouman]

@mjackson I'm not sure that implementing this in ReactRouter only is a great idea. We've discussed this before in #470 (comment)

[tomasklim]

@mjackson hello, when this feature will be ready to use?

[siddhant91]

@mjackson Hello , is there any update on when will this feature be published and ready to be used ?

[cocacrave]

ETA please.

[vladshcherbin]

@cocacrave close to never since this PR is more than 1 year old. The long-standing issue will be 2 years soon. 👏

[cocacrave]

Anybody have a workaround you recommend and use?

[tomasklim]

Somewhere I found this and it works fine:

let lastLocation = null;

history.listen(location => {
  lastLocation = location
});
const prevHistoryPush = history.push;

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

[cocacrave]

@tomasklim Thanks!

[guidobouman]

Hey @mjackson,

If you don't have the time or drive to maintain this package, that's fine.

But might I suggest to let a team of contributors (that are willing to make sure this project stays alive) take care of it then? You could still be the owner, but others could help you out and implement the vision for the project. To shorten the time needed to get a PR merged, or get answers on issues.

[fabe]

Here's another workaround I used a while ago to get around this (in my case, a page transition was called when navigating to the same page). Warning, it's hacky:

const getUserConfirmation = (location, callback) => {
  const [newUrl, action] = location.split('|');

  // Check if user wants to navigate to the same page.
  // If so, we don't want to trigger the page transitions.
  // We have to check the `action`, because the pathnames
  // are the same when going back in history 🤷‍
  const currentUrl = `${window.location.pathname}${window.location.search}`;
  if (newUrl === currentUrl && action === 'PUSH') {
    callback(false);
    return;
  }

  callback(true);
};

// ...

let history = createHistory({ getUserConfirmation });

// `block` must return a string to conform.
// We send both the pathname and action to `getUserConfirmation`.
history.block((location, action) => {
  return `${location.pathname}${location.search}|${action}`;
});

[vladshcherbin]

@guidobouman there is now a roadmap in issues for v4 and 5. However, this issue is not in the list, so I guess we are left with dirty hacks forever. 🤷‍♂️

[vladshcherbin]

For all watchers, there is another try now - #689 (comment) 🚀

[dabit1]

It's little bit annoying that I worked on it and then it's closed with no clear reason.

[mjackson]

Hey everyone, I just realized that I never commented on this issue when I closed it. Sorry about that!

For anyone who is wondering, I left an explanation in another issue about why I decided to leave this functionality out of the history library.

It basically boils down to this: history.push is just a substitute for window.history.pushState. It shouldn't try to be "smart" about doing e.g. a push vs. replace. If it did, we would lose something that the pushState API actually gives us, namely the ability to push the same URL onto the stack twice. There are valid use cases for doing so, and overloading the meaning of "push" in this library would just cause more issues and confusion.

However, the original concern is totally legit. This is why React Router's <Link> component is smart about this in v6 (currently in beta). If you take a look at the source, you'll notice that a <Link> uses a replace instead of a push when the URL is the same. I think this is the right place for this logic.

This way, history + React Router have parity with the DOM. Our history.push works exactly like window.history.pushState and React Router's <Link> works like a normal <a> element.

Again, my apologies for not following up here earlier! I definitely should have.