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

Add a link API that navigates without duplicating paths #617

Closed
wants to merge 1 commit into from

Conversation

sximba
Copy link

@sximba sximba commented Aug 25, 2018

This PR adds a link API that can be used to closely match what browsers do when navigating to the current path, which is to replace instead of push.

I know #570 is already open but I saw no activity on it in a while. I addressed the comments on it.

This fixes a problem that a lot of people have and will allow dependant libraries to close long standing issues.
#470
#558
#507

remix-run/react-router#6137

@sximba sximba force-pushed the master branch 6 times, most recently from 4f7765c to fb34aa6 Compare August 26, 2018 08:29
@mjackson mjackson added this to the 4.8 milestone Nov 1, 2018
@mjackson
Copy link
Member

mjackson commented Nov 1, 2018

Thanks for the PR, @sximba. I definitely want to resolve this in our next release, 4.8. Question: are you also using React Router? Or are you using the history library with something else?

@sximba
Copy link
Author

sximba commented Nov 2, 2018

@mjackson I am using React Router

location.pathname === nextLocation.pathname &&
location.search === nextLocation.search &&
location.hash === nextLocation.hash &&
valueEqual(location.state, nextLocation.state)
Copy link
Contributor

Choose a reason for hiding this comment

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

state shouldn't be compared; replicating how an anchor work only depends on the "visible" location properties.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@sximba sximba force-pushed the master branch 4 times, most recently from 2e4e774 to 9bab68e Compare November 2, 2018 16:34
@gabsprates
Copy link

gabsprates commented Aug 12, 2019

that's great, why this wasn't merged yet?

@mjackson
Copy link
Member

After thinking about this more, this is definitely a problem I want to solve in the router, not here. A <Link> should be smart enough to make the decision to push or replace w/out adding more API here.

I know it's a little frustrating since remix-run/react-router#6137 was already closed and we sent you over here. But thank you for your patience as we sort this out. Let's follow up in remix-run/react-router#5362

@mjackson mjackson closed this Sep 11, 2019
@stefanprobst
Copy link

After thinking about this more, this is definitely a problem I want to solve in the router

could you clarify why not add a way to programmatically do this with history.link? thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Dec 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants