React-router: [native] Add 'component' prop to DOM 'Link'

Created on 3 Mar 2017  路  17Comments  路  Source: ReactTraining/react-router

The native router'sLink component has a different props interface to the DOM package Link. It would be good to align them a little closer, for example, supporting the component prop for the DOM variant. This would a allow people to more easily use the router with something like react-native-web

feature

Most helpful comment

We get this request often but I'm not sure anything will ever convince me to support it.

On the web an <a href/> means something. When you command/control click it, you get a new window, or right click and choose "open in new tab". Screen readers treat them differently, etc.

If we supported <Link component="div"/> not only would we be spitting out invalid markup (<div href/>) we'd also be completely breaking accessibility. To properly turn a non-anchor into a link for a screenreaders you need to

  • add the role="link" attribute
  • add the tabindex="0" attribute
  • register keydown event

    • check in keydown if its the Enter key, if so, navigate

  • register a keyup event

    • check if the keyup is the Space key, if so, navigate

The point of Link in the dom package is to create accessible links that work the way links are supposed to work, not give people a footgun and break how browsers work for all users, not just screen readers. Also, we'd be generating invalid HTML while we were at it.

Now ... that said, your use-case is the first one where I'm like "oh, that's interesting." Any ideas on how to support your goals w/o giving people a footgun?

All 17 comments

Can you not use react-router-native's Link on react-native-web?

You still need a DOM-based router. We'd use this on mobile.twitter.com too

We get this request often but I'm not sure anything will ever convince me to support it.

On the web an <a href/> means something. When you command/control click it, you get a new window, or right click and choose "open in new tab". Screen readers treat them differently, etc.

If we supported <Link component="div"/> not only would we be spitting out invalid markup (<div href/>) we'd also be completely breaking accessibility. To properly turn a non-anchor into a link for a screenreaders you need to

  • add the role="link" attribute
  • add the tabindex="0" attribute
  • register keydown event

    • check in keydown if its the Enter key, if so, navigate

  • register a keyup event

    • check if the keyup is the Space key, if so, navigate

The point of Link in the dom package is to create accessible links that work the way links are supposed to work, not give people a footgun and break how browsers work for all users, not just screen readers. Also, we'd be generating invalid HTML while we were at it.

Now ... that said, your use-case is the first one where I'm like "oh, that's interesting." Any ideas on how to support your goals w/o giving people a footgun?

oh hey ... check this out:

componentDidMount() {
  warning(findDOMNode(this).tagName !== 'A', 'YOU BLEW IT!')
}

s/YOU BLEW IT!/[some actual message about needing to render an anchor]/

I'm not familiar with React Native so I'm curious if this is different -- on web if you want your a to have something other than text inside it that's fine, and ends up working exactly the same in React with react-router's Link:

<Link to="kittens.com">
  <img ... />
</Link>

or:

<Link to="/about">
  <FancyFlexLayoutMenuItemWithDancingIcons>About</...>
</Link>

I've never run into a situation where my ability to style or use a Link depended on bypassing the a wrapper.

Link has a few limitations in the DOM package and they are exaggerated when you're using a library with richer primitives than the DOM provides. For React Native: The default layout doesn't match View or Text; the style interface is incompatible in multiple ways; it's missing propTypes like accessibilityLabel; it doesn't support callbacks like onLayout or onPress*. You can't really work around that in your app either, so you end up copy pasting the internals and making your own Link component/package. Doing so also has the benefit of allowing you to publish components that contain links and work with a certain router context, but don't depend explicitly on RR.

I figured there would be less interest in creating a router-agnostic Link package (haven't looked fully into how Next.js avoids needing a link component, but that's interesting too).

It's certainly a set of complicated trade offs. Warning if the base tag is not an anchor sounds like a reasonable approach. I can get back to you with some specific examples of what we do or want to do

@ryanflorence +1 for warning about using something other than a <a>. I'd like to figure out a way to do it w/out using findDOMNode thought.

@necolas I'd be grateful if you could take a look at our native <Link> and fill in the missing pieces. Our current implementation is pretty basic, but enough to demonstrate that it works.

@mjackson you have refs and you have findDOMNode. I don't know how to use refs to get to the dom node when the component passed in can be several levels of components before getting to the a.

On another note, we don't even know if our props (href, onClick) will make it all the way down.

const Thing = () => <Stuff/>
const Stuff = () => <Other/>
const Other = () => <a onClick={noRouterClick}/>

<Link component={Thing}/> // :\

How do we:

  1. Get a handle on that final tag to check if it's an a? (findDOMNode is the only thing I know of)
  2. How do we make sure out click handler makes it down there? (I don't think we can)

I'd be grateful if you could take a look at our native and fill in the missing pieces.

It's not the native Link he wants changed. The native Link already supports the component prop because there is no platform equivalent to <a>. There are TouchableHighlight, TouchableOpacity, and TouchableWithoutFeedback. TouchableHighlight is the default, but you can pass in the one you want.

What he's asking for is for the dom Link to support it so that he can run the same "native code" codebase on the web. But since the platform semantics around Link vary, I struggle to see how we can accommodate it w/o breaking the platform expectations.

Can't this be solved in react-native-web?

@ryanflorence When @necolas said:

For React Native: The default layout doesn't match View or Text; the style interface is incompatible in multiple ways; it's missing propTypes like accessibilityLabel; it doesn't support callbacks like onLayout or onPress*.

I thought he was critiquing our current native <Link> impl. Maybe I misunderstood?

Oh, sorry, I was reading to hastily :(

Yeah, we need major help on the Link component there in react native!

To clarify: I meant React Native for Web. If you want to run your RN components (that use RR) on the web, you can't because the Link API and layout is different depending on platform. Coming from the other direction: if you want to write your web app itself using RNW, you have a bad time because of the problems I mentioned.

I'll take a look at what an implementation of Link might look like if it's built with the RNW API.

I found this issue while searching for a solution to another valid use case. We use glamorous for theming/styling. With glamorous, to get an anchor tag with appropriate styles we do something like:

const Anchor = glamorous.a(/* ...style code */)

to get a styled component that will still render <a></a> in the DOM. Since there is currently no way to pass this Anchor component to Link we lose all of this styling.

I understand the push for verifying that what gets rendered is in fact an anchor tag, but it would be great to allow some type of customization here.

@lamflam You can do that with glamorous's composition:

const Anchor = glamorous(Link)({ /* ...styles */ })

Closing due to inactivity. I still think the Link component API could be more platform-independent. But you can make your own Link and use that instead of what is in RR

@timdorr Can't believe I missed that, but thank you for pointing it out, works great!

Just following up here: I think making the react-router-dom API consistent with the react-router-native API would actually be really nice. The same issue was brought up in #5437, which I've re-opened. Let's follow up with this discussion there, @necolas.

Was this page helpful?
0 / 5 - 0 ratings