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

fix(gatsby-link): replace current path in history rather than pushing it #23414

Merged

Conversation

abarisain
Copy link
Contributor

Hello!
I'd like to start by thanking you for this awesome project.
Sorry if this contribution doesn't match the guidelines, but I hope I understood them well.

Description

This change makes <GatsbyLink> ask navigate to use replaceState rather than pushState when navigating on a link that is the current page. This fixes an issue where a history item would be pushed multiple times for the current page, while this is not the expected behaviour.

If a developer forced "replace" using the prop, their choice takes priority.

This uses the same technique as @reach/router when they fixed it upstream: reach/router@11e9ed6

I did not copy the

const { key, ...restState } = { ...location.state };
shouldReplace = shallowCompare({ ...state }, restState);

part, because I do not understand it.

I also added some tests.

Note: I did not manage to get unit tests to run on my computer (either by using Node 10, 12, 13 or 14, or even trying on all 3 operating systems). They didn't work in the linked gitpod either.

That said, I tested the changes in gatsby-link using yarn jest in its folder.

Documentation

I don't believe this needs documentation other than updating the changelog

Related Issues

Fixes #22124

Fixes gatsbyjs#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
@abarisain abarisain requested a review from a team as a code owner April 23, 2020 11:00
@ascorbic ascorbic added status: needs core review Currently awaiting review from Core team member topic: react-router* labels May 7, 2020
Copy link
Contributor

@blainekasten blainekasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I tested out the behavior and it works as I would expect! Thanks for the contribution

@blainekasten blainekasten merged commit 5fcd6dc into gatsbyjs:master May 7, 2020
@gatsbot
Copy link

gatsbot bot commented May 7, 2020

Holy buckets, @abarisain — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

@abarisain
Copy link
Contributor Author

abarisain commented May 7, 2020

Thanks! I'm glad to have been able to contribute :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs core review Currently awaiting review from Core team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link should perform a replace when navigating to the current page
3 participants