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

Prevent transitions for external links #4126

Closed

Conversation

sergesemashko
Copy link

Related issue #1147
It seems like many people including myself are ending up creating custom wrapper around <Link/> to handle external link clicks.
I think, ideally that functionality should be handled by react-router as browser has access to current and link's hosts. It turned out that it's relatively easy to compare them on click and prevent router transitions.
Hope this PR will make it and looking forward for the feedback.

@timdorr
Copy link
Member

timdorr commented Nov 1, 2016

No, we specifically do not want to handle external locations in <Link>. You should handle this on your own or just use a normal <a> where appropriate.

@sergesemashko
Copy link
Author

Thanks for reply, I got it.
It would be useful to know the concerns of having this logic rather in common place than in custom workarounds, so others won't ask for it again.
Thanks in advance

@timdorr
Copy link
Member

timdorr commented Nov 1, 2016

It's not the router's job to handle external links. It's only concerned with its internal routing state and the URLs under its control. Also, there are multiple ways in which an external URL could be handled (think server vs client), so its behavior isn't uniform anyways. We should defer to user control so they can choose to handle it however they want (perhaps a window.open or wrapping the link click in an analytics tracking call).

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 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.

2 participants