From 5fcd6dc985d63474e87e3070d6e97baaa0547b39 Mon Sep 17 00:00:00 2001 From: Arnaud Barisain-Monrose Date: Thu, 7 May 2020 21:36:36 +0200 Subject: [PATCH] fix(gatsby-link): replace current path in history rather than pushing it (#23414) Fixes #22124 If the developer didn't explicitly set the "replace" prop and we're on the page that we're about to navigate to, force "replace" to true when navigating. This fixes a bug where clicking a link that does not result in navigating to another page incorrectly adds an history entry. Uses the same technique as @reach/router --- packages/gatsby-link/src/__tests__/index.js | 57 +++++++++++++++++++-- packages/gatsby-link/src/index.js | 8 ++- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/gatsby-link/src/__tests__/index.js b/packages/gatsby-link/src/__tests__/index.js index e28c25808e812..0b1e0930e71f5 100644 --- a/packages/gatsby-link/src/__tests__/index.js +++ b/packages/gatsby-link/src/__tests__/index.js @@ -158,6 +158,53 @@ describe(``, () => { getReplace()(`/some-path`) expect(global.___replace).toHaveBeenCalledWith(`/some-path`) }) + + describe(`uses push or replace adequately`, () => { + it(`respects force disabling replace`, () => { + const to = `/` + getNavigate() + const { link } = setup({ linkProps: { to, replace: false } }) + link.click() + + expect( + global.___navigate + ).toHaveBeenCalledWith(`${global.__BASE_PATH__}${to}`, { replace: false }) + }) + + it(`respects force enabling replace`, () => { + const to = `/courses` + getNavigate() + const { link } = setup({ linkProps: { to, replace: true } }) + link.click() + + expect( + global.___navigate + ).toHaveBeenCalledWith(`${global.__BASE_PATH__}${to}`, { replace: true }) + }) + + it(`does not replace history when navigating away`, () => { + const to = `/courses` + getNavigate() + const { link } = setup({ linkProps: { to } }) + link.click() + + expect(global.___navigate).toHaveBeenCalledWith( + `${global.__BASE_PATH__}${to}`, + {} + ) + }) + + it(`does replace history when navigating on the same page`, () => { + const to = `/` + getNavigate() + const { link } = setup({ linkProps: { to } }) + link.click() + + expect( + global.___navigate + ).toHaveBeenCalledWith(`${global.__BASE_PATH__}${to}`, { replace: true }) + }) + }) }) describe(`withPrefix`, () => { @@ -274,8 +321,12 @@ describe(`state`, () => { const { link } = setup({ linkProps: { state } }) link.click() - expect( - global.___navigate - ).toHaveBeenCalledWith(`${global.__BASE_PATH__}${to}`, { state }) + expect(global.___navigate).toHaveBeenCalledWith( + `${global.__BASE_PATH__}${to}`, + { + replace: true, + state, + } + ) }) }) diff --git a/packages/gatsby-link/src/index.js b/packages/gatsby-link/src/index.js index 53f7147c7962b..6c9dd1fe1986a 100644 --- a/packages/gatsby-link/src/index.js +++ b/packages/gatsby-link/src/index.js @@ -169,9 +169,15 @@ class GatsbyLink extends React.Component { ) { e.preventDefault() + let shouldReplace = replace + const isCurrent = encodeURI(to) === window.location.pathname + if (typeof replace !== `boolean` && isCurrent) { + shouldReplace = true + } + // Make sure the necessary scripts and data are // loaded before continuing. - navigate(to, { state, replace }) + navigate(to, { state, replace: shouldReplace }) } return true