Support full url link

[khankuan]

In some use cases, links can be user inputs. When a generic url is linked to the application, we want to be able to handle the link click through client side navigation instead of full reload.

[khankuan]

21

[fridays]

Thanks for contributing! I like the idea, only the origin part should better happen in a custom component I think. Then you can handle external links differently, for example by using only <a> and possibly target="_blank" like https://github.com/fridays/next-routes/pull/26.

[HaNdTriX]

@khankuan I am not sure If I understand your PR. Do you know about the shallow prop for client side page transitions?

[khankuan]

@HaNdTriX yes, I know about the shallow prop. However, that does not solve my use case. I have a full url (provided by a user input) in an anchor tag. Some of the url points to external sites, i.e, google.com. Some of the url points to a page within my application. For links that point to external sites, I would expect the user to navigate out of my application. For links that point to my application, I want the navigation to be done via client side navigation.

[khankuan]

@fridays sorry for the late response, could you explain a little more about what the external component does?

[HaNdTriX]

@khankuan I can understand your use case, but in order to make it possible we need the origin of the server. This will increase the api surface of this lib. Since this is an optional prop it will be harder to reason about next-routes for new developers.

Its always good for a lib like this to do one thing and do it well. Since you can always use a normal <a> tag for this use case it doesn't justify this api surface increase.

One way to solve your problem, is to use a custom Higher-Order-Component. It could look like this (not tested):

import { Link } from '../routes'

const allowExternal = origin => Component => {
  return class extends Component {
    render () {
      const { href, children, ...others } = this.props
      const isInternal = href && (
        href.indexOf('/') === 0 || 
        href.indexOf(origin) === 0
      )

      if (isInternal) {
        return (
          <Link {...others} href={href} >
            <a>{children}</a>
          </Link>
        )
      }

      return (
        <a {...others} href={href}>
          {children}
        </a>
      )

    }

  }
}

const LinkWithExternal = allowExternal('https://mydomain.com')(Link)

export default LinkWithExternal

Disclaimer: I am not a official maintainer of this package.

[khankuan]

@HaNdTriX Thanks for the explanation. I agree that adding the functionality would increase the scope of the component. I think it is great to apply separation of concerns but I do believe that creating a new component reduces code cohesion.

I wanted to use Link component as the single component in my application to handle hyperlinks. I find it confusing if I need to use different component just to handle hyperlinks that are not defined by the router (such as user inputs, server generated urls etc). Removing the origin prop does not solve the problem if we end up creating a new LinkWithExternal component that would come with the same props plus the origin prop.

The same argument can be applied to the Link component that we have currently. It does 2 things: as and href. Would it be sensible to create a LinkWithAs and LinkWithHref?

I hope libraries maintainers can understand that there is no magic formula to code. Writing imperative code and having the right complexity in your library would help abstract away complexities from applications that use your library from the nice declarative api that you have provided.

Thank you.

[oliviertassinari]

I'm not sure that piece of logic should belong in this repository. I would rather see it implemented by next.js or on user-land. Having it here seems brittle. Here is my user-land implementation:

Link.js

```jsx import React from 'react' import PropTypes from 'prop-types' import classNames from 'classnames' import { Link as LinkRouter } from 'frontend/src/routes' import { withStyles, createStyleSheet } from 'material-ui/styles' const styleSheet = createStyleSheet('Link', theme => ({ root: { color: 'inherit', textDecoration: 'inherit', '&:hover': { textDecoration: 'underline', }, }, primary: { color: theme.palette.primary[500], }, button: { '&:hover': { textDecoration: 'inherit', }, }, })) function Link(props) { const { children: childrenProp, component: ComponentProp, classes, className: classNameProp, variant, route, ...other } = props let Component const className = classNames( classes.root, { [classes.primary]: variant === 'primary', [classes.button]: variant === 'button', }, classNameProp, ) const more = {} let children = childrenProp if (ComponentProp) { Component = ComponentProp more.className = className } else if (route) { Component = LinkRouter more.route = route children = {children} } else { Component = 'a' more.className = className } return ( {children} ) } Link.propTypes = { children: PropTypes.node.isRequired, classes: PropTypes.object.isRequired, className: PropTypes.string, component: PropTypes.oneOfType([PropTypes.string, PropTypes.func]), route: PropTypes.string, variant: PropTypes.oneOf(['primary', 'button']), } export default withStyles(styleSheet)(Link) ```