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 #23380

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

abarisain
Copy link
Contributor

@abarisain abarisain commented Apr 22, 2020

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.

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

That said, I tested the changes locally in my project by building gatsby-link manually.
For what it's worth, this change didn't add any more test failures than I already had.
I hope the CI can build this.

Documentation

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

Related Issues

Fixes #22124

@abarisain abarisain requested a review from a team as a code owner April 22, 2020 16:41
@abarisain
Copy link
Contributor Author

I'll have a look at why the tests are failing

… itFixes #22124If the developer didn't explicitly set the "replace" prop and we're onthe page that we're about to navigate to,force "replace" to truewhen navigating.This fixes a bug where clicking a link that does not result innavigating to another page incorrectlyadds an history entry.Using the same technique as @reach/router
@abarisain abarisain closed this Apr 23, 2020
@abarisain abarisain requested review from a team as code owners April 23, 2020 10:24
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 this pull request may close these issues.

Link should perform a replace when navigating to the current page
1 participant