Feature request: 'to' property as a function in Link component

[smashercosmo]

In RR v3 you could pass a function to 'to' property in Link component. Would you be interested in porting this functionality to RR v4? If yes I would be happy to submit a PR.

[timdorr]

What's the use case for this? It sounds like you can achieve the same thing with component composition.

[smashercosmo]

Yep, it's just for convenience and for reducing the amount of API inconsistencies between v3 and v4

[timdorr]

Well, go ahead and work up a PR. We can discuss the implementation specifics over there.

[mjackson]

FWIW I don't see a lot of value in this feature. Seems like you could just as easily call the function yourself instead of asking us to call it.

<Link to={someFunc}/>
<Link to={someFunc(stuff)}/>

In the first implementation we have to decide what arguments we're going to pass to the function you provide. In the second, you just go ahead and call it.

[smashercosmo]

mmm... but the point is that this function is called with "location" as argument. In RR3 it was very convenient, that you didn't have wrap parent component in withRouter to get the location object.

<Link to={location => {...location, query: {search: 'hello'}}}>

Of course in RR4 you can achieve that by wrapping Link into Route component, but I just thought it would be nice to keep the API consistent between RR3 and RR4.

[mjackson]

Ah, I see. That's a great use case, thanks for sharing :). I'd definitely be open to a PR that does this.

[smashercosmo]

I started working on the PR. @timdorr could you give me some guidance? Currently I have a problem with tests. When I run them I get a lot of errors like "Cannot find module 'react-router' from 'BrowserRouter.js'". What could be the problem?

[timdorr]

You most likely need to run the build script first.

[smashercosmo]

@timdorr thx, it worked. One more question. I noticed, that you're not currently testing clicks on a Link. Should I write these tests using react test utils? And if yes, should I write them only for to-as-a-function case or for all the cases?

[smashercosmo]

@timdorr ping

[pshrmn]

@smashercosmo There used to be some tests, but I think that they got lost in the switch to the monorepo. I have a click test in the relative link PR that you can reference: https://github.com/pshrmn/react-router/blob/cd42cf65a2a7394bd6b234238f370df8d274ca80/packages/react-router-dom/modules/__tests__/Link-test.js#L127-L154

I would probably start with just the test for to-as-a-function. The other tests should probably be brought back, but maybe in another PR?

[smashercosmo]

@pshrmn ok, thx, got it

[smashercosmo]

@timdorr @pshrmn One more question, I'm not quite sure what to do with NavLink, because in NavLink "to" property value gets passed to ''path" property of Route component.

[pshrmn]

@smashercosmo The way that I would approach <NavLink> is to call the to function right away to generate the location object to navigate to. You will also need to add context to the <NavLink> in order to access the current location.

const NavLink = ({ to, ... }, context) => {
  const goTo = typeof to === 'function'
    ? to(props.location || context.router.route.location)
    : to
  return (
    <Route
      ...
      children={(...) => {
        <Link to={goTo} ... />
      }
    />
  )
}

NavLink.contextTypes = {
  router: PropTypes.shape({
    route: PropTypes.shape({
      location: PropTypes.object.isRequired
    }).isRequired
  }).isRequired
}

[pshrmn]

Also, feel free to open a PR. You can always rebase later, so it doesn't really hurt to open the PR early.

[smashercosmo]

@pshrmn Alright, thank you very much

[smashercosmo]

Done #5368

[jimrandomh]

It looks like this issue fell through the cracks and didn't get merged. This is causing porting problems for us (LessWrong); we're upgrading react-router-dom because our framework (Vulcan) dragged us along, and there are some existing usages of the function-style links in our code. It's too late for these PRs to help us, but, could one of them get merged reasonably soonish?