Rework <Link> behavior (backwards compatible) (#36436)

Fixes https://github.com/vercel/next.js/discussions/32233

⚠️ If you're looking at this PR please read the complete description including the part about incremental adoption.

## TLDR:

Official support for `<Link href="/about">About</Link>` / `<Link href="/about"><CustomComponent /></Link>` / `<Link href="/about"><strong>About</strong></Link>` where `<Link>` always renders `<a>` without edge cases where it doesn’t render `<a>`. You'll no longer have to put an empty `<a>` in `<Link>` with this enabled.

## Full context

### Changes to `<Link>`

- Added an `legacyBehavior` prop that defaults to `true` to preserve the defaults we have today, this will allow to run a codemod on existing codebases to move them to the version where `legacyBehavior` becomes `false` by default
- When using the new behavior `<Link>` always renders an `<a>` instead of having `React.cloneElement` and passing props onto a child element
- When using the new behavior props that can be passed to `<a>` can be passed to `<Link>`. Previously you could do something like `<Link href="/somewhere"><a target="_blank">Download</a></Link>` but with `<Link>` rendering `<a>` it now allows these props to be set on link. E.g. `<Link href="/somewhere" target="_blank"></Link>` / `<Link href="/somewhere" className="link"></Link>`

### Incremental Adoption / Codemod

The main reason we haven't made these changes before is that it breaks pretty much all Next.js apps, which is why I've been hesitant to make this change in the past. I've spent a bunch of time figuring out what the right approach is to rolling this out and ended up with an approach that requires existing apps to run a codemod that automatically opts their `<Link>` usage into the old behavior in order to keep the app functioning.

This codemod will auto-fix the usage where possible. For example: 

- When you have `<Link href="/about"><a>About</a></Link>` it'll auto-fix to `<Link href="/about">About</Link>`
- When you have `<Link href="/about"><a onClick={() => console.log('clicked')}>About</a></Link>` it'll auto-fix to `<Link href="/about" onClick={() => console.log('clicked')}>About</Link>`
- For cases where auto-fixing can't be applied the `legacyBehavior` prop is added. When you have `<Link href="/about"><Component /></Link>` it'll transform to `<Link href="/about" legacyBehavior><Component /></Link>` so that your app keeps functioning using the old behavior for that particular link. It's then up to the dev to move that case out of the `legacyBehavior` prop.


**This default will be changed in Next.js 13, it does not affect existing apps in Next.js 12 unless opted in via `experimental.newLinkBehavior` and running the codemod.**

Some code samples of what changed:

```jsx
const CustomComponent = () => <strong>Hello</strong>

// Legacy behavior: `<a>` has to be nested otherwise it's excluded

// Renders: <a href="/about">About</a>. `<a>` has to be nested.
<Link href="/about">
  <a>About</a>  
</Link>

// Renders: <strong onClick={nextLinkClickHandler}>Hello</strong>. No `<a>` is included.
<Link href="/about">
  <strong>Hello</strong>
</Link>


// Renders: <strong onClick={nextLinkClickHandler}>Hello</strong>. No `<a>` is included.
<Link href="/about">
  <CustomComponent />
</Link>

// --------------------------------------------------
// New behavior: `<Link>` always renders `<a>`

// Renders: <a href="/about">About</a>. `<a>` no longer has to be nested.
<Link href="/about">
  About
</Link>

// Renders: <a href="/about"><strong>Hello</strong></a>. `<a>` is included.
<Link href="/about">
  <strong>Hello</strong>
</Link>

// Renders: <a href="/about"><strong>Hello</strong></a>. `<a>` is included.
<Link href="/about">
  <CustomComponent />
</Link>
```

---


## Feature

- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`


Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
This commit is contained in:
Tim Neutkens 2022-04-26 00:01:30 +02:00 committed by GitHub
parent b264064ad5
commit 489e65ed98
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
27 changed files with 532 additions and 44 deletions

View file

@ -120,6 +120,10 @@ const TRANSFORMER_INQUIRER_CHOICES = [
name: 'cra-to-next (experimental): automatically migrates a Create React App project to Next.js',
value: 'cra-to-next',
},
{
name: 'new-link: Ensures your <Link> usage is backwards compatible. Used in combination with experimental newNextLinkBehavior',
value: 'new-link',
},
]
function expandFilePathsIfNeeded(filesBeforeExpansion) {

View file

@ -0,0 +1,13 @@
import Link from 'next/link'
function Comp({children}) {
return children
}
export default function Page() {
return (
<Link href="/">
<Comp>Home</Comp>
</Link>
);
}

View file

@ -0,0 +1,13 @@
import Link from 'next/link'
function Comp({children}) {
return children
}
export default function Page() {
return (
<Link href="/" legacyBehavior>
<Comp>Home</Comp>
</Link>
);
}

View file

@ -0,0 +1,16 @@
import Link from 'next/link'
function Comp({children}) {
return children
}
const a = <Comp />
export default function Page() {
return (
<Link href="/about">
{a}
</Link>
);
}

View file

@ -0,0 +1,15 @@
import Link from 'next/link'
function Comp({children}) {
return children
}
const a = <Comp />
export default function Page() {
return (
<Link href="/about" legacyBehavior>
{a}
</Link>
);
}

View file

@ -0,0 +1,9 @@
import Link from 'next/link'
export default function Page() {
return (
<Link href="/" legacyBehavior>
<a>Home</a>
</Link>
)
}

View file

@ -0,0 +1,9 @@
import Link from 'next/link'
export default function Page() {
return (
<Link href="/" legacyBehavior>
<a>Home</a>
</Link>
)
}

View file

@ -0,0 +1,8 @@
import Link from 'next/link'
export default function Page() {
return (
<Link href="/about">
<a>Link</a>
</Link>
);
}

View file

@ -0,0 +1,8 @@
import Link from 'next/link'
export default function Page() {
return (
<Link href="/about">
Link
</Link>
);
}

View file

@ -0,0 +1,10 @@
import Link from 'next/link'
export default function Page() {
return (
<Link href="/about">
<a onClick={() => {
console.log('clicked')
}} download>Link</a>
</Link>
)
}

View file

@ -0,0 +1,13 @@
import Link from 'next/link'
export default function Page() {
return (
<Link
href="/about"
onClick={() => {
console.log('clicked')
}}
download>
Link
</Link>
);
}

View file

@ -0,0 +1,8 @@
import Link from 'next/link'
const linkProps = {}
export default function Page() {
return <Link href="/about"><a className="link" {...linkProps}>about</a></Link>;
}

View file

@ -0,0 +1,7 @@
import Link from 'next/link'
const linkProps = {}
export default function Page() {
return <Link href="/about" className="link" {...linkProps}>about</Link>;
}

View file

@ -0,0 +1,21 @@
/* global jest */
jest.autoMockOff()
const defineTest = require('jscodeshift/dist/testUtils').defineTest
const fixtures = [
'link-a',
'move-props',
'add-legacy-behavior',
'excludes-links-with-legacybehavior-prop',
'children-interpolation',
'spread-props'
]
for (const fixture of fixtures) {
defineTest(
__dirname,
'new-link',
null,
`new-link/${fixture}`
)
}

View file

@ -0,0 +1,72 @@
import { API, FileInfo } from 'jscodeshift'
export default function transformer(file: FileInfo, api: API) {
const j = api.jscodeshift
const $j = j(file.source)
return $j
.find(j.ImportDeclaration, { source: { value: 'next/link' } })
.forEach((path) => {
const defaultImport = j(path).find(j.ImportDefaultSpecifier)
if (defaultImport.size() === 0) {
return
}
const variableName = j(path)
.find(j.ImportDefaultSpecifier)
.find(j.Identifier)
.get('name').value
if (!variableName) {
return
}
const linkElements = $j.findJSXElements(variableName)
linkElements.forEach((linkPath) => {
const $link = j(linkPath).filter((childPath) => {
// Exclude links with `legacybehavior` prop from modification
return (
j(childPath)
.find(j.JSXAttribute, { name: { name: 'legacyBehavior' } })
.size() === 0
)
})
if ($link.size() === 0) {
return
}
// Direct child elements referenced
const $childrenElements = $link.childElements()
const $childrenWithA = $childrenElements.filter((childPath) => {
return (
j(childPath).find(j.JSXOpeningElement).get('name').get('name')
.value === 'a'
)
})
// No <a> as child to <Link> so the old behavior is used
if ($childrenWithA.size() !== 1) {
$link
.get('attributes')
.push(j.jsxAttribute(j.jsxIdentifier('legacyBehavior')))
return
}
const props = $childrenWithA.get('attributes').value
const hasProps = props.length > 0
if (hasProps) {
// Add props to <Link>
$link.get('attributes').value.push(...props)
// Remove props from <a>
props.length = 0
}
//
const childrenProps = $childrenWithA.get('children')
$childrenWithA.replaceWith(childrenProps.value)
})
})
.toSource()
}

View file

@ -1369,6 +1369,9 @@ export default async function getBaseWebpackConfig(
'process.env.NODE_ENV': JSON.stringify(
dev ? 'development' : 'production'
),
'process.env.__NEXT_NEW_LINK_BEHAVIOR': JSON.stringify(
config.experimental.newNextLinkBehavior
),
'process.env.__NEXT_CROSS_ORIGIN': JSON.stringify(crossOrigin),
'process.browser': JSON.stringify(targetWeb),
'process.env.__NEXT_TEST_MODE': JSON.stringify(

View file

@ -20,7 +20,7 @@ type OptionalKeys<T> = {
[K in keyof T]-?: {} extends Pick<T, K> ? K : never
}[keyof T]
export type LinkProps = {
type InternalLinkProps = {
href: Url
as?: Url
replace?: boolean
@ -29,9 +29,21 @@ export type LinkProps = {
passHref?: boolean
prefetch?: boolean
locale?: string | false
legacyBehavior?: boolean
/**
* requires experimental.newNextLinkBehavior
*/
onMouseEnter?: (e: React.MouseEvent) => void
/**
* requires experimental.newNextLinkBehavior
*/
onClick?: (e: React.MouseEvent) => void
}
export type LinkProps = InternalLinkProps &
React.AnchorHTMLAttributes<HTMLAnchorElement>
type LinkPropsRequired = RequiredKeys<LinkProps>
type LinkPropsOptional = OptionalKeys<LinkProps>
type LinkPropsOptional = OptionalKeys<InternalLinkProps>
const prefetched: { [cacheKey: string]: boolean } = {}
@ -105,6 +117,9 @@ function linkClicked(
}
function Link(props: React.PropsWithChildren<LinkProps>) {
const {
legacyBehavior = Boolean(process.env.__NEXT_NEW_LINK_BEHAVIOR) !== true,
} = props
if (process.env.NODE_ENV !== 'production') {
function createPropError(args: {
key: string
@ -154,6 +169,9 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
passHref: true,
prefetch: true,
locale: true,
onClick: true,
onMouseEnter: true,
legacyBehavior: true,
} as const
const optionalProps: LinkPropsOptional[] = Object.keys(
optionalPropsGuard
@ -177,12 +195,21 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
actual: valType,
})
}
} else if (key === 'onClick' || key === 'onMouseEnter') {
if (props[key] && valType !== 'function') {
throw createPropError({
key,
expected: '`function`',
actual: valType,
})
}
} else if (
key === 'replace' ||
key === 'scroll' ||
key === 'shallow' ||
key === 'passHref' ||
key === 'prefetch'
key === 'prefetch' ||
key === 'legacyBehavior'
) {
if (props[key] != null && valType !== 'boolean') {
throw createPropError({
@ -208,39 +235,68 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
)
}
}
const p = props.prefetch !== false
let children: React.ReactNode
const {
href: hrefProp,
as: asProp,
children: childrenProp,
prefetch: prefetchProp,
passHref,
replace,
shallow,
scroll,
locale,
onClick,
onMouseEnter,
...restProps
} = props
children = childrenProp
if (legacyBehavior && typeof children === 'string') {
children = <a>{children}</a>
}
const p = prefetchProp !== false
const router = useRouter()
const { href, as } = React.useMemo(() => {
const [resolvedHref, resolvedAs] = resolveHref(router, props.href, true)
const [resolvedHref, resolvedAs] = resolveHref(router, hrefProp, true)
return {
href: resolvedHref,
as: props.as ? resolveHref(router, props.as) : resolvedAs || resolvedHref,
as: asProp ? resolveHref(router, asProp) : resolvedAs || resolvedHref,
}
}, [router, props.href, props.as])
}, [router, hrefProp, asProp])
const previousHref = React.useRef<string>(href)
const previousAs = React.useRef<string>(as)
let { children, replace, shallow, scroll, locale } = props
if (typeof children === 'string') {
children = <a>{children}</a>
}
// This will return the first child, if multiple are provided it will throw an error
let child: any
if (legacyBehavior) {
if (process.env.NODE_ENV === 'development') {
if (onClick) {
console.warn(
`"onClick" was passed to <Link> with \`href\` of \`${hrefProp}\` but "legacyBehavior" was set. The legacy behavior requires onClick be set on the child of next/link`
)
}
if (onMouseEnter) {
console.warn(
`"onMouseEnter" was passed to <Link> with \`href\` of \`${hrefProp}\` but "legacyBehavior" was set. The legacy behavior requires onMouseEnter be set on the child of next/link`
)
}
try {
child = React.Children.only(children)
} catch (err) {
if (!children) {
throw new Error(
`No children were passed to <Link> with \`href\` of \`${props.href}\` but one child is required https://nextjs.org/docs/messages/link-no-children`
`No children were passed to <Link> with \`href\` of \`${hrefProp}\` but one child is required https://nextjs.org/docs/messages/link-no-children`
)
}
throw new Error(
`Multiple children were passed to <Link> with \`href\` of \`${props.href}\` but only one child is supported https://nextjs.org/docs/messages/link-multiple-children` +
`Multiple children were passed to <Link> with \`href\` of \`${hrefProp}\` but only one child is supported https://nextjs.org/docs/messages/link-multiple-children` +
(typeof window !== 'undefined'
? " \nOpen your browser's console to view the Component stack trace."
: '')
@ -249,7 +305,10 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
} else {
child = React.Children.only(children)
}
const childRef: any = child && typeof child === 'object' && child.ref
}
const childRef: any =
legacyBehavior && child && typeof child === 'object' && child.ref
const [setIntersectionRef, isVisible, resetVisible] = useIntersection({
rootMargin: '200px',
@ -265,14 +324,14 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
}
setIntersectionRef(el)
if (childRef) {
if (legacyBehavior && childRef) {
if (typeof childRef === 'function') childRef(el)
else if (typeof childRef === 'object') {
childRef.current = el
}
}
},
[as, childRef, href, resetVisible, setIntersectionRef]
[as, childRef, href, resetVisible, setIntersectionRef, legacyBehavior]
)
React.useEffect(() => {
const shouldPrefetch = isVisible && p && isLocalURL(href)
@ -288,7 +347,7 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
}, [as, href, isVisible, locale, p, router])
const childProps: {
onMouseEnter?: React.MouseEventHandler
onMouseEnter: React.MouseEventHandler
onClick: React.MouseEventHandler
href?: string
ref?: any
@ -302,27 +361,45 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
)
}
}
if (child.props && typeof child.props.onClick === 'function') {
if (!legacyBehavior && typeof onClick === 'function') {
onClick(e)
}
if (
legacyBehavior &&
child.props &&
typeof child.props.onClick === 'function'
) {
child.props.onClick(e)
}
if (!e.defaultPrevented) {
linkClicked(e, router, href, as, replace, shallow, scroll, locale)
}
},
onMouseEnter: (e: React.MouseEvent) => {
if (!legacyBehavior && typeof onMouseEnter === 'function') {
onMouseEnter(e)
}
childProps.onMouseEnter = (e: React.MouseEvent) => {
if (child.props && typeof child.props.onMouseEnter === 'function') {
if (
legacyBehavior &&
child.props &&
typeof child.props.onMouseEnter === 'function'
) {
child.props.onMouseEnter(e)
}
if (isLocalURL(href)) {
prefetch(router, href, as, { priority: true })
}
},
}
// If child is an <a> tag and doesn't have a href attribute, or if the 'passHref' property is
// defined, we specify the current 'href', so that repetition is not needed by the user
if (props.passHref || (child.type === 'a' && !('href' in child.props))) {
if (
!legacyBehavior ||
passHref ||
(child.type === 'a' && !('href' in child.props))
) {
const curLocale =
typeof locale !== 'undefined' ? locale : router && router.locale
@ -343,7 +420,13 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
addBasePath(addLocale(as, curLocale, router && router.defaultLocale))
}
return React.cloneElement(child, childProps)
return legacyBehavior ? (
React.cloneElement(child, childProps)
) : (
<a {...restProps} {...childProps}>
{children}
</a>
)
}
export default Link

View file

@ -76,6 +76,7 @@ export interface NextJsWebpackConfig {
}
export interface ExperimentalConfig {
newNextLinkBehavior?: boolean
disablePostcssPresetEnv?: boolean
swcMinify?: boolean
swcFileReading?: boolean
@ -464,6 +465,8 @@ export const defaultConfig: NextConfig = {
staticPageGenerationTimeout: 60,
swcMinify: false,
experimental: {
// TODO: change default in next major release (current v12.1.5)
newNextLinkBehavior: false,
cpus: Math.max(
1,
(Number(process.env.CIRCLE_NODE_TOTAL) ||

View file

@ -0,0 +1,5 @@
module.exports = {
experimental: {
newNextLinkBehavior: true,
},
}

View file

@ -0,0 +1,9 @@
import Link from 'next/link'
export default function Page() {
return (
<>
<h1 id="about-page">About Page</h1>
<Link href="/">Home</Link>
</>
)
}

View file

@ -0,0 +1,8 @@
import Link from 'next/link'
export default function Page() {
return (
<Link href="/" className="home-link">
Home
</Link>
)
}

View file

@ -0,0 +1,8 @@
import Link from 'next/link'
export default function Page() {
return (
<Link href="/" id="home-link">
Home
</Link>
)
}

View file

@ -0,0 +1,9 @@
import Link from 'next/link'
export default function Page() {
return (
<>
<h1>Home Page</h1>
<Link href="/about">About</Link>
</>
)
}

View file

@ -0,0 +1,11 @@
import Link from 'next/link'
export default function Page() {
return (
<>
<h1>Home Page</h1>
<Link href="/about">
About <strong>Additional Children</strong>
</Link>
</>
)
}

View file

@ -0,0 +1,17 @@
import Link from 'next/link'
export default function Page() {
return (
<>
<h1>Onclick prevent default</h1>
<Link
href="/"
onClick={(e) => {
e.preventDefault()
console.log('link to home clicked but prevented')
}}
>
Home
</Link>
</>
)
}

View file

@ -0,0 +1,13 @@
import Link from 'next/link'
export default function Page() {
return (
<Link
href="/"
onClick={() => {
console.log('link to home clicked')
}}
>
Home
</Link>
)
}

View file

@ -0,0 +1,93 @@
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { renderViaHTTP } from 'next-test-utils'
import webdriver from 'next-webdriver'
import cheerio from 'cheerio'
import path from 'path'
async function matchLogs(browser, includes: string) {
let found = false
const browserLogs = await browser.log('browser')
browserLogs.forEach((log) => {
if (log.message.includes(includes)) {
found = true
}
})
return found
}
const appDir = path.join(__dirname, './app')
describe('New Link Behavior', () => {
let next: NextInstance
beforeAll(async () => {
next = await createNext({
files: {
pages: new FileRef(path.join(appDir, 'pages')),
'next.config.js': new FileRef(path.join(appDir, 'next.config.js')),
},
dependencies: {},
})
})
afterAll(() => next.destroy())
it('should render link with <a>', async () => {
const html = await renderViaHTTP(next.url, '/')
const $ = cheerio.load(html)
const $a = $('a')
expect($a.text()).toBe('About')
expect($a.attr('href')).toBe('/about')
})
it('should navigate to /about', async () => {
const browser = await webdriver(next.url, `/`)
await browser.elementByCss('a').click().waitForElementByCss('#about-page')
const text = await browser.elementByCss('h1').text()
expect(text).toBe('About Page')
})
it('should handle onclick', async () => {
const browser = await webdriver(next.url, `/onclick`)
await browser.elementByCss('a').click().waitForElementByCss('h1')
const text = await browser.elementByCss('h1').text()
expect(text).toBe('Home Page')
expect(await matchLogs(browser, 'link to home clicked')).toBe(true)
})
it('should handle preventdefault', async () => {
const browser = await webdriver(next.url, `/onclick-prevent-default`)
await browser.elementByCss('a').click()
const text = await browser.elementByCss('h1').text()
expect(text).toBe('Onclick prevent default')
expect(await matchLogs(browser, 'link to home clicked but prevented')).toBe(
true
)
})
it('should render link with id', async () => {
const html = await renderViaHTTP(next.url, '/id-pass-through')
const $ = cheerio.load(html)
const $a = $('a')
expect($a.attr('id')).toBe('home-link')
})
it('should render link with classname', async () => {
const html = await renderViaHTTP(next.url, '/classname-pass-through')
const $ = cheerio.load(html)
const $a = $('a')
expect($a.attr('class')).toBe('home-link')
})
it('should render link with multiple children', async () => {
const html = await renderViaHTTP(next.url, '/multiple-children')
const $ = cheerio.load(html)
const $a = $('a')
expect($a.text()).toBe('About Additional Children')
expect($a.find('strong').text()).toBe('Additional Children')
})
})