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

Use replace instead of push on <Link> click when location is the same #5362

Closed
jasonmacgowan opened this issue Jul 21, 2017 · 28 comments · Fixed by #7864
Closed

Use replace instead of push on <Link> click when location is the same #5362

jasonmacgowan opened this issue Jul 21, 2017 · 28 comments · Fixed by #7864

Comments

@jasonmacgowan
Copy link

jasonmacgowan commented Jul 21, 2017

Each time you click a <Link/>, it pushes to the browser history even if the location already matches. This differs from the default implementation of anchor tags as far as I can tell.

When a simple anchor tag is clicked 100 times, there is only one history entry
<a href="/foo">Go to Foo</a>

If you did the same for <Link />, you have 100 history entries which makes back and forward button navigation difficult.

Proposing we don't push when the location already matches

@mahesh-beehively
Copy link

Same issue react-router-native

@pnest
Copy link

pnest commented Aug 4, 2017

Is it an issue? You can check link yorself and set appropriate 'replace' attribiute value in

@timdorr
Copy link
Member

timdorr commented Aug 4, 2017

Edit: I have seen the abundance of thumbs downs and acknowledge my stupidity here. Feel free to ignore this comment...

@pnest has the right idea. This is "fixable" with the replace prop.

The point of <Link> is to emulate <a>. A normal <a> that links to the same page you are already on will push new history entities to the stack. For instance, click the react-router link at the top of the page a bunch of times. You'll see that you have to click back multiple times to get back to this page. We want to maintain HTML semantics with our defaults, rather than reinvent them.

@timdorr timdorr closed this as completed Aug 4, 2017
@pshrmn
Copy link
Contributor

pshrmn commented Aug 4, 2017

@timdorr I actually think that the behavior of that link is caused by GitHub's SPA working incorrectly. Quoting myself from here:

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))

Using the replace prop can work, but I think that it would be better if history identified when navigating to the same location and replaces instead of pushes. That was the behavior in v2/3, but it was lost in the history rewrite.

Since this is a history issue, this doesn't need to be reopened. If/when history is updated to add same location detection, the <Link> API can be updated but for the time being replace is good enough.

@vladshcherbin
Copy link
Contributor

vladshcherbin commented Aug 6, 2017

@timdorr actually, normal same page <a> links don't create duplicates, so current <Link /> behavior is wrong and differs from 99% of websites. Even RR3 was working correctly, only RR4 is not.

@pnest this is a problem because it differs from default behavior, you have to reinvent Link component in every project because of RR4 (and history v4) bug.

This is a really annoying bug in RR4.

@remix-run remix-run deleted a comment from wub Sep 5, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
@remix-run remix-run unlocked this conversation Aug 16, 2019
@mjackson
Copy link
Member

Re-opening this issue because this is still broken. Going to add this to the roadmap for our next minor release.

@mjackson mjackson reopened this Aug 16, 2019
@mjackson mjackson changed the title Each <Link/> click pushes to history, even when location is the same Use replace instead of push on <Link> click when location is the same Aug 16, 2019
@pshrmn
Copy link
Contributor

pshrmn commented Aug 16, 2019

I think that there could be a temp fix in a minor release of React Router, but the best long term approach would be for history to handle this automatically (e.g. with a method that compares the new location with the current location to determine if it pushes/replaces).

@guidobouman
Copy link
Contributor

guidobouman commented Aug 19, 2019

You want to fix this in react-router, okay.

But is there maybe a path for other libraries that have have a concept of showing screens (like modals) to also integrate programatically with the same history stack that react-router uses?

Something along the lines of:

import { RouterAPI } from "react-router-dom";

// optional
const options = {
  replace: false,
  // ...
}

RouterAPI.go('...', options);
RouterAPI.link('...', options);
RouterAPI.navigate('...', options);
RouterAPI.emitLinkClick('...', options);
RouterAPI.immitateLinkClick('...', options);

@pshrmn
Copy link
Contributor

pshrmn commented Aug 19, 2019

@guidobouman React Router uses the history package for its navigation.

@guidobouman
Copy link
Contributor

guidobouman commented Aug 19, 2019

@pshrmn I know. I have built this functionality on top of history for previous projects. I have also been advocating the adoption of link in history for some years: remix-run/history#470 (comment). But that's going against @mjackson's philosophies for the package. If we can't convince him in three years time to put it inside history, then I'm willing to adjust my approaches to something that might actually get merged and still fix the scenario I'm trying to solve.

@stale
Copy link

stale bot commented Oct 18, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Oct 18, 2019
@guidobouman
Copy link
Contributor

@mjackson If this is the tracking ticket for this feature I would like to prevent this from becoming stale. Any ETA on 5.2?

@stale stale bot removed the stale label Oct 18, 2019
@TrevorSayre
Copy link

@guidobouman @mjackson maybe we could consider updating https://github.com/ReactTraining/react-router/blob/master/.github/stale.yml to include a label in the exemptLabels section such as roadmap. Issues that have been accepted as something to be worked on can be labeled as roadmap and will not be marked as stale (and possibly closed) unless the roadmap label is removed.

@cloudever
Copy link

I just use history.block API always following way:

function applyBrowserLocationBlocker(history: History) {
  let currentLocation = null

  history.block((location, action) => {
    const nextLocation = location.pathname + location.search

    if (action === 'PUSH') {
      if (currentLocation === nextLocation) {
        return false
      }
    }

    currentLocation = nextLocation
  })
}

@ghost
Copy link

ghost commented Nov 13, 2019

I'm with @guidobouman I thought that was the intent of not completely owning the history responsibility, exposing historys state. I do agree that non-state changing reloads of the same page route should be consolidated if at all automatable, atleast with a flag to use replace for those that want their hands off the navigation completely. The logic could live safely in Link to verify before dispatching.

@ghost
Copy link

ghost commented Nov 13, 2019

I want to be clear I am satisfied with this behavior and would prefer any changes to be behind a flag rather than having to lock at version, are there any others seeing the current behavior as a feature rather than a bug? This package was a life saver and it deserves appriciation. I love the conversations I see over here. Keep up the great work!

@stale stale bot added the stale label Jan 12, 2020
bors bot referenced this issue in foosbyte/kicker-trainer Oct 19, 2021
1431: fix: update react monorepo to v17.0.2 (master) r=renovate[bot] a=renovate[bot]

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [react](https://reactjs.org/) ([source](https://togithub.com/facebook/react)) | [`17.0.1` -> `17.0.2`](https://renovatebot.com/diffs/npm/react/17.0.1/17.0.2) | [![age](https://badges.renovateapi.com/packages/npm/react/17.0.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react/17.0.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react/17.0.2/compatibility-slim/17.0.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react/17.0.2/confidence-slim/17.0.1)](https://docs.renovatebot.com/merge-confidence/) |
| [react-dom](https://reactjs.org/) ([source](https://togithub.com/facebook/react)) | [`17.0.1` -> `17.0.2`](https://renovatebot.com/diffs/npm/react-dom/17.0.1/17.0.2) | [![age](https://badges.renovateapi.com/packages/npm/react-dom/17.0.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-dom/17.0.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-dom/17.0.2/compatibility-slim/17.0.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-dom/17.0.2/confidence-slim/17.0.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>facebook/react</summary>

### [`v17.0.2`](https://togithub.com/facebook/react/blob/master/CHANGELOG.md#&#8203;1702-March-22-2021)

[Compare Source](https://togithub.com/facebook/react/compare/v17.0.1...v17.0.2)

##### React DOM

-   Remove an unused dependency to address the [`SharedArrayBuffer` cross-origin isolation warning](https://developer.chrome.com/blog/enabling-shared-array-buffer/). ([`@&#8203;koba04](https://togithub.com/koba04)` and [`@&#8203;bvaughn](https://togithub.com/bvaughn)` in [#&#8203;20831](https://togithub.com/facebook/react/pull/20831), [#&#8203;20832](https://togithub.com/facebook/react/pull/20832), and [#&#8203;20840](https://togithub.com/facebook/react/pull/20840))

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/foosbyte/kicker-trainer).

1432: fix: update react-router monorepo (master) r=renovate[bot] a=renovate[bot]

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [react-router](https://togithub.com/ReactTraining/react-router) | [`5.2.0` -> `5.2.1`](https://renovatebot.com/diffs/npm/react-router/5.2.0/5.2.1) | [![age](https://badges.renovateapi.com/packages/npm/react-router/5.2.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-router/5.2.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-router/5.2.1/compatibility-slim/5.2.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-router/5.2.1/confidence-slim/5.2.0)](https://docs.renovatebot.com/merge-confidence/) |
| [react-router-dom](https://togithub.com/ReactTraining/react-router) | [`5.2.0` -> `5.3.0`](https://renovatebot.com/diffs/npm/react-router-dom/5.2.0/5.3.0) | [![age](https://badges.renovateapi.com/packages/npm/react-router-dom/5.3.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-router-dom/5.3.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-router-dom/5.3.0/compatibility-slim/5.2.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-router-dom/5.3.0/confidence-slim/5.2.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>ReactTraining/react-router</summary>

### [`v5.2.1`](https://togithub.com/ReactTraining/react-router/releases/v5.2.1)

[Compare Source](https://togithub.com/ReactTraining/react-router/compare/v5.2.0...v5.2.1)

This release fixes a bug with `<Link>` so that, when the `to` location is the same as the current, the history state entry is replaced instead of pushed to the stack. See [https://github.com/remix-run/react-router/issues/5362](https://togithub.com/remix-run/react-router/issues/5362) for details. 🥳

Thanks to [`@&#8203;guidobouman](https://togithub.com/guidobouman)` for the PR and for everyone else who weighed in for the fix!

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/foosbyte/kicker-trainer).

Co-authored-by: Renovate Bot <bot@renovateapp.com>
kodiakhq bot referenced this issue in sullivanpj/open-system Jun 26, 2023
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [react-router](https://togithub.com/remix-run/react-router) | [`5.2.0` -> `5.3.4`](https://renovatebot.com/diffs/npm/react-router/5.2.0/5.3.4) | [![age](https://badges.renovateapi.com/packages/npm/react-router/5.3.4/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-router/5.3.4/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-router/5.3.4/compatibility-slim/5.2.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-router/5.3.4/confidence-slim/5.2.0)](https://docs.renovatebot.com/merge-confidence/) |
| [react-router-dom](https://togithub.com/remix-run/react-router) | [`5.2.0` -> `5.3.4`](https://renovatebot.com/diffs/npm/react-router-dom/5.2.0/5.3.4) | [![age](https://badges.renovateapi.com/packages/npm/react-router-dom/5.3.4/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-router-dom/5.3.4/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-router-dom/5.3.4/compatibility-slim/5.2.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-router-dom/5.3.4/confidence-slim/5.2.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>remix-run/react-router (react-router)</summary>

### [`v5.3.4`](https://togithub.com/remix-run/react-router/releases/tag/v5.3.4)

[Compare Source](https://togithub.com/remix-run/react-router/compare/v5.3.3...v5.3.4)

We removed the `mini-create-react-context` dependency, moving it into an internal module to eliminate peer dependency warnings for users on React 18 ([#&#8203;9382](https://togithub.com/remix-run/react-router/issues/9382)).

**Full Changelog**: remix-run/react-router@v5.3.3...v5.3.4

### [`v5.3.3`](https://togithub.com/remix-run/react-router/releases/tag/v5.3.3)

[Compare Source](https://togithub.com/remix-run/react-router/compare/v5.3.2...v5.3.3)

This release fixes a bad version selector in react-router-native.

### [`v5.3.2`](https://togithub.com/remix-run/react-router/releases/tag/v5.3.2)

[Compare Source](https://togithub.com/remix-run/react-router/compare/v5.3.1...v5.3.2)

-   Fix: make v5 Router compatible with v18 StrictMode by [@&#8203;jgoz](https://togithub.com/jgoz) in [https://github.com/remix-run/react-router/pull/8831](https://togithub.com/remix-run/react-router/pull/8831)

### [`v5.3.1`](https://togithub.com/remix-run/react-router/releases/tag/v5.3.1)

[Compare Source](https://togithub.com/remix-run/react-router/compare/v5.2.1...v5.3.1)

This release adds missing `LICENSE` files to the published build.

### [`v5.2.1`](https://togithub.com/remix-run/react-router/releases/tag/v5.2.1)

[Compare Source](https://togithub.com/remix-run/react-router/compare/v5.2.0...v5.2.1)

This release fixes a bug with `<Link>` so that, when the `to` location is the same as the current, the history state entry is replaced instead of pushed to the stack. See [https://github.com/remix-run/react-router/issues/5362](https://togithub.com/remix-run/react-router/issues/5362) for details. 🥳

Thanks to [@&#8203;guidobouman](https://togithub.com/guidobouman) for the PR and for everyone else who weighed in for the fix!

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/sullivanpj/open-system).
This was referenced Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.