Consider handling onClick on next/link elements

[seidtgeist]

Currently next/link will ignore onClick props set on <Link> or contained <a> elements:

import Link from 'next/link'

const customHandleClick = () => console.log('Free pizza!')

// Somewhere in render-land

<Link href='/' onClick={customHandleClick}>
  <a>No free pizza :(</a>
</Link>

<Link href='/'>
  <a onClick={customHandleClick}>No free pizza :/</a>
</Link>

Clicking either of those two links will not call the customHandleClick function.

This is because the onClick handler of the contained <a> tag is overridden by the next/link implementation here.

If this were to be changed a good way would be checking for, and calling, the onClick prop passed to the <Link> element as part of its internal click handler. See react-router’s Link element for inspiration.

Why I would consider changing this: onClick handlers on links can be useful for various things, but mostly I didn’t expect that the onClick wasn’t being called at all. I’ve added support for this to my custom Link element, but I think onClick "should just work".

[arunoda]

This is not a good API generally. Because it's not sure what you could do with that handler since the page is already changing to the new route.

I get the point of onClick and why you need to use that.
That's why we've the next/router API. Use that.

[seidtgeist]

@arunoda The onClick handler would mainly be useful for things like firing off an analytics tracking call. What about all the great things declarative next/link elements do that the next/router API doesn’t do? Anyway, @janv helped me find a wicked way to prevent onClick from being overwritten and still being able to use next/link, so I’ll do that 😄

[janv]

Not a good API generally is: Silently overriding mechanisms of an established API, subverting users expectations and preventing default behavior.

Consider a solution like this (in <Link/>):

linkClicked(e){
  const child = Children.only(this.props.children)
  if (child.props.onClick) child.props.onClick(e)
  if (e.defaultPrevented) return
  // do your thing ...
}

render(){
  // irrelevant parts omitted ...
  const child = Children.only(children)
  const props = {
    onClick: this.linkClicked
  }
  return React.cloneElement(child, props)
}

[ianmartorell]

@ehd do you mind sharing that wicked way? :)

[seidtgeist]

Courtesy of the great @janv:

import Link from 'next/link'
import React from 'react'

// This is the custom wrapper component you would build and use just like `next/link`:

class MyLink extends React.Component {
  render () {
    const { onCustomClick, ...props } = this.props
    return <a {...props} onClick={this.handleClick} />
  }

  handleClick = event => {
    if (this.props.onClick) {
      this.props.onClick(event)
    }

    if (this.props.onCustomClick) {
      this.props.onCustomClick(event)
    }
  }
}

// This is some example where you would use `MyLink` including a click handler

export default class SomeComponent extends React.Component {
  render () {
    return <div>
      <Link href='/hello'>
        <MyLink onCustomClick={() => alert('lol')}>
          LinkText
        </MyLink>
      </Link>
    </div>
  }
}

[oliviertassinari]

Because it's not sure what you could do with that handler since the page is already changing to the new route.

@arunoda I have another use case with the documentation of Material-UI. I want to close the Drawer right after the user link interaction, waiting for the new page to load. I'm gonna use the component middleware hack for now (I believe that the issue should be opened).

[aaronsmulktis]

ahh bollocks i couldn't get the wickedness to work...

here's how I'm doing it unsuccessfully:

import Link from 'next/link'; import MyLink from './myLink';

with a good ol

<Link href="/"><MyLink onCustomClick={this.logout}>Logout</MyLink></Link>

he'll I even tried

<MyLink onCustomClick={this.logout}>Logout</MyLink> all by its lonesome and, although slightly different behavior, yielded not this.logout

[CorwinCZ]

I too think this should behave naturally, here is my work-around that doesn't need custom component:

<Link href='/hello'>
  <div>
    <a onClick={() => {customFuntionCall()}}>
      LinkText
    </a>
  </div>
</Link>

Since <Link> adds onClick handler to first children, we can nest them inside each other and use native event bubling. Two thinks to keep in mind - the <div> element (or any other in it's place) can't be removed and the custom onClick handler can't call event.stopPropagation() or the <Link> would stop working.

Hope this helps 😄

[buesing]

This has tripped me up a couple times already. I agree that there needs to be a non-hacky way to attach click handlers to links. Analytics tracking is a common use case. At the very least a warning should be thrown, instead of just silently overriding it.

[alexanbj]

I would also like to see this issue reopened. In my case I have a dropdown with links, and I want to close the dropdown when clicking on a link. I can get work around it, but proper support for onClick would be nice!

[kunokdev]

I think this is especially useful if the <Link /> redirects to the same page again.

[fabioespinosa]

It can also be useful to show a loader on screen (this.setState({loading: true}) while the other page loads

[viniciusCamargo]

Here's what I've been doing:

import React, { Component } from 'react'
import Router from 'next/router'

export default class MyLink extends Component {
  componentDidMount () {
    Router.prefetch(this.props.url)
  }

  render() {
    const { url, asUrl, children, onClick } = this['props']

    return (
      <a href={asUrl} onClick={(event) => {
        event.preventDefault()
        
        onClick()
        Router.push(url, asUrl)
      }}>
        {children}
      </a>
    )
  }
}

I'm not sure about performance issues though.

[paschalidi]

@viniciusCamargo how are you handling command+click/ctrl+click behaviour then?

[viniciusCamargo]

Hey @paschalidi, unfortunately, I'm not handling it, I didn't have the time to try to fix it yet.

Edit: btw, if you have any idea for a fix I'd appreciate.

[riyasat-ali]

@arunoda what if we have to implement some validations on onClick function before login?

[keeth]

Link is so much more powerful than Router (preloading, smart handling in linkClicked) that it's painful to have to duplicate that logic outside of Link to make this work. I have the same use case as @oliviertassinari - I need to close my Material UI drawer :)

@CorwinCZ's workaround is great. Just need to add a {/*Do not remove this div.*/} comment to it. :)

[oliviertassinari]

@keeth Here is the workaround (hack) we use: OnClick.

[ckeeney]

CorwinCZ's work around works, but I had problems with the URL not showing in the browser when you hover over the link, and also styling was slightly more complicated (my links were inline elements and the div he added is a block element). I adapted the solution to the following, which resolves both of those problems:

export const NavLink = ({page, params, onClick=()=>{}, children}) => {
    return (
        <li>
            <Link {...Router.linkPage(page, params)} passHref prefetch>
                    <a>
                        <span onClick={onClick}>
                            {children}
                        </span>
                    </a>
            </Link>
        </li>
    )
};

This shows the pointer by default when mousing over a link and doesn't create block elements in your links.

Edited: Removed redundant functional wrapper as suggested by @JonAbrams.

[JonAbrams]

Thanks @ckeeney, your solution worked great for me, but I simplified my span like so: <span onClick={onClick}>.

One remaining small downside is the event.currentTarget will be a span instead of an a. It'd be awesome if next.js adopted @janv's solution

[typeofweb]

Should this be reopened?

[julkue]

Guess it should based on the many upvotes.

[typeofweb]

@julmot it's implemented now: #4474

[timneutkens]

It's already out on canary, so re-opening is not needed. https://github.com/zeit/next.js/releases

[seidtgeist]

Thank you all!