Skip to content

Commit

Permalink
Require component rendered as child of Link to pass event to `onCli…
Browse files Browse the repository at this point in the history
…ck` handler (vercel#27723)

Currently, if you render `next/link` like

```ts
<Link>
  <CustomComponent />
</Link>
```

and have 

```tsx
interface Props {
  onClick?: () => void; // <— note we're not passing event as an argument
}

function CustomComponent({ onClick }: Props) {
  return <div onClick={() => onClick?.()}>Hello</div>
}
```

It'll result in error

```
link.js?f421:21 Uncaught TypeError: Cannot read property 'defaultPrevented' of undefined
```

<!--
Thanks for opening a PR! Your contribution is much appreciated.
In order to make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below.
Choose the right checklist for the change that you're making:
-->

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes


Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
  • Loading branch information
2 people authored and natew committed Feb 16, 2022
1 parent 8d0972a commit 023a01f
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 0 deletions.
7 changes: 7 additions & 0 deletions packages/next/client/link.tsx
Expand Up @@ -284,6 +284,13 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
} = {
ref: setRef,
onClick: (e: React.MouseEvent) => {
if (process.env.NODE_ENV !== 'production') {
if (!e) {
throw new Error(
`Component rendered inside next/link has to pass click event to "onClick" prop.`
)
}
}
if (child.props && typeof child.props.onClick === 'function') {
child.props.onClick(e)
}
Expand Down
35 changes: 35 additions & 0 deletions test/integration/client-navigation/pages/link-invalid-onclick.js
@@ -0,0 +1,35 @@
import Link from 'next/link'
import { useState } from 'react'

export default function Page(props) {
const [errorCount, setErrorCount] = useState(0)

function Button(props) {
return (
<a
id="custom-button"
href={props.href}
onClick={(e) => {
e.preventDefault()
try {
props.onClick()
} catch (err) {
setErrorCount(errorCount + 1)
console.error(err)
}
}}
>
{props.href}
</a>
)
}

return (
<>
<p id="errors">{errorCount}</p>
<Link href="/nav" passHref>
<Button />
</Link>
</>
)
}
7 changes: 7 additions & 0 deletions test/integration/client-navigation/test/index.test.js
Expand Up @@ -87,6 +87,13 @@ describe('Client Navigation', () => {
expect(text).toMatch(/this is the about page/i)
})

it('should error when calling onClick without event', async () => {
const browser = await webdriver(context.appPort, '/link-invalid-onclick')
expect(await browser.elementByCss('#errors').text()).toBe('0')
await browser.elementByCss('#custom-button').click()
expect(await browser.elementByCss('#errors').text()).toBe('1')
})

it('should navigate via the client side', async () => {
const browser = await webdriver(context.appPort, '/nav')

Expand Down

0 comments on commit 023a01f

Please sign in to comment.