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

<Link> does not understand external URL with protocol #1147

Closed
davidtheclark opened this issue Apr 30, 2015 · 20 comments
Closed

<Link> does not understand external URL with protocol #1147

davidtheclark opened this issue Apr 30, 2015 · 20 comments
Labels

Comments

@davidtheclark
Copy link

If I use <Link to='//google.com'> it's fine; but if I use <Link to='http://google.com'> I get an error: Uncaught Error: Invariant Violation: Cannot find a route named "http://google.com".

Based on the docs, I believe the <Link> component is supposed to handle full URLs as well as routes, so this appears to be a bug.

@agundermann
Copy link
Contributor

I guess the docs are wrong. Instead of full URLs it should say full paths. For external links, just use <a href>.

@davidtheclark
Copy link
Author

If that's the case, then (with docs clarified) this issue still stands but as a feature request rather than a bug. (Also, it's confusing that the external links work without a protocol?)

Let's say I have an array of objects representing 20 links, a few of which are "routes" and a few of which are true URLs. I want to map those to components and render them. If <Link> doesn't understand how to deal with both, then I need to perform the logic myself that distinguishes between external URLs and routes and renders different elements accordingly.

Seems it would be nicer for react-router users if the provided <Link> did this for them. For the situation described above, I'd probably have to end up writing another component wrapping this logic and rendering either a <Link> or an <a> accordingly --- don't other react-router users want the same thing?

Or if other people know of a good existing way (or a better non-existing way) to take care of that use-case, I'd love to hear it. (e.g. page.js hijacks any <a> clicks and checks whether they apply to routes: https://github.com/visionmedia/page.js/blob/master/index.js#L539)

@ryanflorence
Copy link
Member

<Link/> is a router-aware anchor, if you want to link to an external site, use an <a/>.

@ryanflorence
Copy link
Member

Note, you could make your own Link that does the check and then renders an <a/> or a <Link/>

var YourLink = React.createClass({
  render () {
    return this.isExternal() ? <a/> : <Link/>
  }
});

@geraldfullam
Copy link

geraldfullam commented May 26, 2016

I agree with @davidtheclark . I just ran into this issue and began implementing my own component similar to the one you describe above to handle the logic, but it seemed so basic and obvious a concern and such a common scenario that I stopped to check the react-router docs and issues for the "built-in" solution that I was certain existed. I am surprised that this feature is not built-in.

@shprink
Copy link

shprink commented Jun 15, 2016

I think everyone that uses React-Router end up writing a component like that. Good or Bad IDK but annoying indeed :P

@shprink
Copy link

shprink commented Jun 15, 2016

I created something like this if it helps: https://gist.github.com/shprink/bf9599e1d66b9dc4d151e89c1199ccb8

@robbyemmert
Copy link

I agree with @shprink and @davidtheclark. If you want to handle dynamic routes (which nearly every app does), you'll have to add this functionality. Is there any reason that a URL with a protocol and a different domain should NOT be honored? @ryanflorence, would you merge a PR if I added this functionality for you?

@taion
Copy link
Contributor

taion commented Jun 16, 2016

As currently set up this wouldn't be the correct place to put this logic – this ought instead to be handled by the history.

@joepio
Copy link

joepio commented Mar 2, 2017

Thanks for the tip, @ryanflorence. I ended up writing this component to handle both:

import React, { Component, PropTypes } from 'react';
import { Link } from 'react-router';
import isExternal from 'is-url-external';

const propTypes = {
  to: PropTypes.string.isRequired,
};

/**
 * Link that also works for external URL's
 */
export default class LinkDuo extends Component {
  render() {
    return isExternal(this.props.to) ?
      <a
        href={this.props.to}
        {...this.props}
      />
      :
      <Link {...this.props} />;
  }
}

LinkDuo.propTypes = propTypes;

@saadaouad
Copy link

Try to use this condition directly

{/^https?:\/\//.test(url)
? <a href={url} />
: <Link to={url} />
}

@joncursi
Copy link

I wasn't able to use the is-url-external because my application needs to render server-side. Instead, I used the validator package to check if the to prop is an external URL or not:

/* @flow */

import * as React from 'react';
import { Link as LinkImport } from 'react-router-dom';
import validator from 'validator';

type PropsFlowType = {
  to: string | Object,
};

const Link = ({
  to,
  ...otherProps
}: PropsFlowType): React.Node => (
  validator.isURL(to)
    ? (
      <a
        href={to}
        {...otherProps}
      />
    ) : (
      <LinkImport
        to={to}
        {...otherProps}
      />
    )
);

export default Link;

@mshwery
Copy link

mshwery commented Jan 2, 2018

This should probably be covered in the <Link /> component documentation. It is completely unexpected behavior right now.

The docs are misleading through omission – they say that a location object can be provided as the to prop, but do not mention that the protocol and origin properties within that location object are ignored, or that a full url string as the to prop will be applied relatively: href="http://a.example.com/http://b.example.com" instead of href="http://b.example.com"

@aluethi
Copy link

aluethi commented Jan 14, 2018

Since I needed a TypeScript implementation with as few external dependencies as possible I made this drop in replacement of the Link class in react-router-dom (with the help of all your comments). It might be of help to some of you as well:

import * as React from 'react';
import { Link as RouterLink } from 'react-router-dom';

export default class Link extends React.Component<any, {}> {
    constructor(props: any) {
        super(props);
    }

    render(): JSX.Element {
        return /^https?:\/\//.test(this.props.to)
            ? <a href={this.props.to} {...this.props} />
            : <RouterLink {...this.props} />
    }
}

@arttay
Copy link

arttay commented Jan 16, 2018

+1 for either adding this functionality or at min adding it to the docs. Spent a day trying to figure out why the app wouldn't link to external sources.

@ghost
Copy link

ghost commented Feb 5, 2018

My solution, including a no to option, an external link and an internal link:

// What <Link> should always be
import React from 'react';
import { Link } from 'react-router-dom';

export default ({ to, children, ...props }) => {

  // It is a simple element with nothing to link to
  if (!to) return <span {...props}>{children}</span>;

  // It is intended to be an external link
  if (/^https?:\/\//.test(to)) return <a href={to} {...props}>{children}</a>;

  // Finally, it is an internal link
  return <Link to={to} {...props}>{children}</Link>;
};

Cannot remove that children (and let react use the props.children from {...props}) because a11y complains about the <a> not having a manual children even if they have from the {...props}.

@mattwills8
Copy link

mattwills8 commented Feb 5, 2018

I created a render props component which you pass the url, a renderInternal function, and a renderExternal function as props.

It then checks if the link is internal or external, and calls either renderInternal or renderExternal. That way there's no messing around with trying to pass through any React-Router props, you can just pass your usual React-Router component to renderInternal

Looks a bit like this:

class Prefix_Link extends Component {

  render () {

    // this is a function that for me either returned the url with host removed or returned false if it was an external url
    const internal = removeHostFromUrl(this.props.url) 

    return internal ? this.props.renderInternal(internal) : this.props.renderExternal(this.props.url)
  }
}

 Prefix_Link.propTypes = {
    url: PropTypes.string.isRequired,
    renderInternal: PropTypes.func.isRequired,
    renderExternal: PropTypes.func.isRequired
}

export default Prefix_Link

example usage:

<BCorpLink
    url={link}
    renderInternal={url => {
      return (
        <Link to={url} replace >
          {button}
        </Link>
      )
    }}
    renderExternal={url => {
      return (
        <a href={url} >
          {button}
        </a>
      )
    }} />

If gives you complete flexibility over any classes or props you want to pass to or .

Hope that helps!

@michalpleszczynski

This comment has been minimized.

@remix-run remix-run locked as resolved and limited conversation to collaborators Apr 16, 2018
@mjackson
Copy link
Member

mjackson commented Nov 2, 2018

Let's re-open this until we can add some notes to the docs about using external links and add a warning for URLs with anything more than just a path in <Link>.

@mjackson mjackson reopened this Nov 2, 2018
@mjackson
Copy link
Member

mjackson commented Sep 8, 2021

Actually, I'm thinking we just make <Link> work transparently for same-origin URLs and external ones. That way you don't have to remember when to use <Link> and when to use <a>, so the abstraction is a little more complete.

We should get this fixed in v5 and make sure it works in v6 too.

@timdorr timdorr closed this as completed Dec 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests