Skip to content

Commit

Permalink
fix(gatsby-link): replace current path in history rather than pushing…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
abarisain committed May 7, 2020
1 parent 8e6d5c6 commit 5fcd6dc
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 4 deletions.
57 changes: 54 additions & 3 deletions packages/gatsby-link/src/__tests__/index.js
Expand Up @@ -158,6 +158,53 @@ describe(`<Link />`, () => {
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`, () => {
Expand Down Expand Up @@ -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,
}
)
})
})
8 changes: 7 additions & 1 deletion packages/gatsby-link/src/index.js
Expand Up @@ -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
Expand Down

0 comments on commit 5fcd6dc

Please sign in to comment.