React-router: HOC for Link aka withLink

[marzelin]

With the rise of styling libraries like glamorous and styled-components styling component become so much better. But those libraries share a weakness:
If I want to style a third-party component like Link I can do it conveniently but I have to trust that the third-party component knows how to handle className prop passed to it by a styling library.

Styling libraries have no way to know if a third-party component uses className properly, thus fail silently in the case of misuse giving no info whatsoever. That can lead to pretty nasty bugs.

Also Link breaks the container/persentational separation. It both provides some data and displays something on the page.

I'd like to propose adding HOC for Link that would solve this problem.
With withLink I could do this:

import A from '.styledElements/A'
import withLink from 'react-router-dom/withLink'

const MyLink = withLink((props) => <A {...props}/>) // A is a styled component

This gives me 100% certainty that the styles will be applied and also gives me freedom to choose how I want to present links in my app.

The implementation should be straightforward:

// withLink.js is just slightly changed Link.js
const withLink = (Comp) => class Link extends React.Component {
   // ... nothing changes here
  render() {
    const { replace, to, ...props } = this.props // eslint-disable-line no-unused-vars
    const onClick = this.handleClick
    const href = this.context.router.history.createHref(
      typeof to === 'string' ? { pathname: to } : to
    )
    const passProps = {...props, onClick, href}


    return <Comp {...passProps} />
  }
}

export default withLink

// Link.js
import React from 'react'
import withLink from './withLink'

export default withLink((props) => <a {...props}/>)

[timdorr]

This might be a better answer to #4625

[pshrmn]

This is nice in theory, but I don't think that Ryan wants to include any official way to make non-anchor links. I wouldn't mind it, but only with suitable messaging to say that it should only be used with components that render anchors.

Styled components are the only good reason that rendering custom components is desirable, but there isn't a good way to ensure that the component renders an anchor. When non-anchors become "hyperlinks", we end up with either poor accessibility or needing to added a whole bunch of event logic (including manually implementing the shift and ctrl new window/tab opening logic) just to replicate what an anchor does already.

I think that the best approach for now would be to release a separate package (react-router-styled-link?) that does this.

[marzelin]

I understand your concerns that boil down to: "don't give people option that can only do harm" but as I explained in my initial comment I need this feature to make working with css-in-js libraries reliable. I care a lot about predictable code so I'd be grateful for implementing this feature.

BTW Why do you think that the first thing people will do when given this option is replace anchors with something else? That would be immensely silly thing to do. (most) Developers =/= reckless teenagers. :)

[timdorr]

<button>s, yo. Definitely see that happening.

[diondiondion]

I'd like to propose adding HOC for Link that would solve this problem.

But Link passes through the className prop properly, so it isn't really a problem in this specific case, is it?

[wanbinkimoon]

@diondiondion I think this issue is referred to custom props, not className directly
@marzelin I love this solution, I'm experiencing the same problem, and I love your solution, but I do not understand why there should be a difference with the standard Link component.

[nathanforce]

@timdorr @ryanflorence Any more thoughts here? We have a similar need where we have a custom <Anchor /> component that abstracts away things like click tracking but still renders an underlying <a> tag. Would an HOC with a warning for non <a> children ease your concerns of misuse?

[mjackson]

Let me try and interpret this: you're upset because you're not the one who gets to decide what props are passed to the <a>, is that right? Looking at the current implementation, we just pass all extra props straight on through to the <a>, so this shouldn't be a problem. You have complete control over the presentation layer, besides the fact that you must render an <a>.

[marzelin]

Briefly summarizing:

1. Current implementation can lead to bugs that can easily slip to production.
2. Current implementation is against single responsibility principle.
3. There's resistance towards more flexible implementation just because some individual may misuse it on their own website.

Have a nice day.

[mjackson]

@marzelin I didn't mean to offend you. I was honestly trying to understand your concern. Sorry if it came across as rude. We get a lot of issues on this repo and it's difficult to give each one a lot of time and attention.

That being said, react-router-dom's <Link> implementation won't ever render anything besides an <a> if that's what you're asking. <Link> is an anchor tag. It's not a general purpose navigator component. The click handler in <Link> is very specific to DOM/<a> concerns.

[marzelin]

@mjackson I didn't feel offended. I'm glad there's finally a clear decision concerning this issue, although I don't agree with it. Let's move on. :man_technologist:

[mjackson]

Cool, thanks @marzelin.

[kachkaev]

I support @marzelin's request to add a linkHOC as I also struggle when using third-party UI libraries with react-router. Antd's component called <Button> can perfectly work like a link (e.g. START SURVEY), so as the Material UI's <ListItem> (e.g. GO TO DASHBOARD). The only option that I see at the moment is in _inheriting_ the Link, putting <Button /> or whatever inside the new component and naming all this <LinkButton/>. Example here: https://github.com/ReactTraining/react-router/issues/4463#issuecomment-284172389.

A hoc link that passes a few props to its child would be a really useful part of react-router IMO; at least the creation of <LinkButton/> would become not so verbose.

[mjackson]

You can use a regular <Route> to create your <LinkButton>.

const LinkButton = ({ to }) => (
  <Route render={({ history }) => (
    <button onClick={() => history.push(to)}/>
  )}/>
)

Or, if you prefer to use HOC:

const LinkButton = withRouter(({ to, history }) => (
  <button onClick={() => history.push(to)}/>
))

[kachkaev]

I'm afraid that I can't agree that it's the same. When you inherit <Link/>, handleClick is aware of the modifier keys, so it can open a custom button or a menu item in a new tab or window. Just calling history.push() flattens the navigation, which may frustrate a user quite badly.

Just imagine filling in a huge form and then clicking on a FAQ menu item with ctrl, thus expecting to read something helpful before you’ve clicked _submit_. The link opens in the current tab instead and so the last 15 minutes of your life have just been wasted ⛈ ⛈ ⛈

[mjackson]

@kachkaev My example code was only to illustrate that you have access to the imperative history API so you can do whatever you'd like.

[kachkaev]

@mjackson yeah I know I can surround history.push() with anything I want. However, the most common scenario will be just rewriting Link's handleClick() by hand. This suggests that many can benefit from having a withLink HOC directly from react-router :–)

If you are afraid of any HOC misuse that may affect a11y, a bold warning in the docs will enough for the majority of the sane developers. Insane ones will find out how to do UX badly anyway :–)

[pshrmn]

Not every React Router aware component has to live in the react-router/react-router-dom packages. If you really want this, your best bet is to make your own package. The source is basically just wrapping the existing Link.js code for the withLink function.

function withLink(Component) {
  return class Link extends React.Component {
    // copy most of the code from Link.js, just modifying the render
    render() {
      // ...
      return <Component ... />
    }
  }
}

or modifying the <Link> to take a component prop if you prefer that approach.

class Link extends React.Component {
  // copy...
  render() {
    // ...
    const { component:Component = 'a', ... } = this.props;
    return <Component ... />
  }
}

Maybe toss in a few tests, figure out what to call the package, npm publish it and you're good to go.

[mjackson]

Thanks for pointing this out @pshrmn. Probably the best thing about our API being just components is that the integration with React Router is very straightforward. You usually don't need any API from us. Just write your own component and off you go.

[wanbinkimoon]

I think that, at least, should be evaluated the chance to use the link
component as a styled component and pass props to it in order to change the
aspect without create another just-presentational wrapper component
On Sat, 15 Jul 2017 at 22:45, MICHAEL JACKSON [e-mail]
wrote:

Thanks for pointing this out @pshrmn https://github.com/pshrmn.
Probably the best thing about our API being just components is that the
integration with React Router is very straightforward. You usually don't
need any API from us. Just write your own component and off you go.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/ReactTraining/react-router/issues/5182#issuecomment-315561485,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJns17qMHX7Ko_HvP5H6iEbR_Q9Wy1NIks5sOST5gaJpZM4No9Ki
.

>

Nicola Bertelloni // [phone]

[ncuillery]

Despites the fact that React is all about composition instead of inheritance, I've solved this problem in very few lines of code:

import { Button } from 'antd';
import { Link } from 'react-router-dom';

class ButtonLink extends Link {

  render() {
    const linkMarkup = super.render();
    return <Button {...linkMarkup.props} />;
  }

}

Button is a React component which renders a HTML looking like a button if the props href is provided. See https://ant.design/components/button/

I guess it works with any other component like your styled-component.