Allow <Link> to have a custom logic for active state

[smashercosmo]

Along with @taion's purposal #3657, I suggest to also allow component to have a prop, which accepts function for implementing custom isActive logic. It could look something like this

<Link 
  to="/hello"
  isActive={location => location.pathname === '/hello'} 
  activeClassName="active">
  Hello
</Link>

[taion]

I'm not a fan of this – it seems simpler for "active state" to be a property of the router and the link's location descriptor. It doesn't seem useful for there to be multiple concepts of "active" beyond what we already have.

[smashercosmo]

This can effectively solve #3293 without introducing breaking changes

[taion]

I think the right thing there is to just accept active as a prop. That way the user can do whatever.

Alternatively, to allow a custom concept of "active" at the level of the router.

It's weird to have a <Link> component specifically have custom active semantics. The to as a function is just to have a convenient way of building links that e.g. only update the query or the hash.

[smashercosmo]

Yes, but if 'active' is a simple boolean prop then we still face the issue of passing down location, if Link is deeply nested. Why not allow this prop to accept function as well?

[taion]

Because "active" semantics shouldn't usually be link-specific. If you want links to be active on exact matches, or on prefix matches, we should allow configuring that globally – it shouldn't be link-by-link.

[smashercosmo]

Imagine, you have a search page with a search filter. And every change to a filter reflects in the location query. But you want search page link in nav bar to stay active no mater what the query is. But all other links in the application should keep the normal matching behaviour. I think that's not the exotic case. And configuring active logic on router level won't solve this problem.

I know, that this was discussed a lot before. But this solution is quite simple and I can't really see the drawbacks. Could be that I don't see the whole picture, but in that case could you point out things, that I miss.

[taion]

We already have handling for that, though – we ignore query parameters in active calculations, except to verify that the ones specified are a subset of the ones that are active.

[taion]

The point isn't that this is awful or anything – it's just that it's expanded API surface area, in such a way that I don't think pays for itself in allowing common idioms the way letting someone write:

<Link to={location => ({ ...location, hash: '#the-hash' })}>

does.

[smashercosmo]

Ok, feel free to close. Unless you you want to wait for other maintainers opinion. Anyway thx for discussion.

[timdorr]

We pass along any provided props, so you can do this outside of the component (perhaps in a wrapper for reusability).

[tachang]

@taion Can you elaborate on "we ignore query parameters in active calculations, except to verify that the ones specified are a subset of the ones that are active."

I am changing the query string via

    self.props.router.replace({
      'pathname': window.location.pathname,
      'query': self.state.filterValues
    });

And the Link I have is

<Link to={`${Constants.APP_URL_PREFIX}/dashboard?${this.state.filterQueryString}`} activeClassName="active" >Dashboard</Link>

But the active class is not being set. The URL in my Chrome browser is exactly the same as if I were to right click and do copy link.

[taion]

<Link
  to={{
    pathname: newPathname,
    query: newQuery,
  }}
>

[tachang]

@taion Thanks that worked! For reference I did this:

                    <Link to={
                      {
                        'pathname': `/dashboard`,
                        'query': this.state.filterValues
                      }
                    } activeClassName="active" onClick={this.handleClick}>Dashboard</Link>