Skip to content

Commit

Permalink
App directory next/link dynamic href dev error (#43074)
Browse files Browse the repository at this point in the history
Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
Fixes #42715
  • Loading branch information
hanneslund committed Nov 25, 2022
1 parent c16d4be commit 0f195f0
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 0 deletions.
36 changes: 36 additions & 0 deletions errors/app-dir-dynamic-href.md
@@ -0,0 +1,36 @@
# Dynamic `href` is not supported in the `/app` directory

#### Why This Error Occurred

You have tried to use a dynamic `href` with `next/link` in the `app` directory. This is not supported as the new client-side router no longer uses a mapping of dynamic routes -> url, instead it always leverages the url.

#### Possible Ways to Fix It

**Before**

```jsx
<Link
href={{
pathname: '/route/[slug]',
query: { slug: '1' },
}}
>
link
</Link>
```

Or

```jsx
<Link href="/route/[slug]?slug=1">link</Link>
```

**After**

```jsx
<Link href="/route/1">link</Link>
```

### Useful Links

[`next/link` documentation](https://beta.nextjs.org/docs/api-reference/components/link#href)
4 changes: 4 additions & 0 deletions errors/manifest.json
Expand Up @@ -765,6 +765,10 @@
{
"title": "experimental-app-dir-config",
"path": "/errors/experimental-app-dir-config.md"
},
{
"title": "app-dir-dynamic-href",
"path": "/errors/app-dir-dynamic-href.md"
}
]
}
Expand Down
26 changes: 26 additions & 0 deletions packages/next/client/link.tsx
Expand Up @@ -397,6 +397,32 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
// We're in the app directory if there is no pages router.
const isAppRouter = !pagesRouter

if (process.env.NODE_ENV !== 'production') {
if (isAppRouter && !asProp) {
let href: string | undefined
if (typeof hrefProp === 'string') {
href = hrefProp
} else if (
typeof hrefProp === 'object' &&
typeof hrefProp.pathname === 'string'
) {
href = hrefProp.pathname
}

if (href) {
const hasDynamicSegment = href
.split('/')
.some((segment) => segment.startsWith('[') && segment.endsWith(']'))

if (hasDynamicSegment) {
throw new Error(
`Dynamic href \`${href}\` found in <Link> while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href`
)
}
}
}
}

const { href, as } = React.useMemo(() => {
if (!pagesRouter) {
const resolvedHref = formatStringOrUrl(hrefProp)
Expand Down
75 changes: 75 additions & 0 deletions test/e2e/app-dir/dynamic-href.test.ts
@@ -0,0 +1,75 @@
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { getRedboxDescription, hasRedbox } from 'next-test-utils'
import path from 'path'
import webdriver from 'next-webdriver'

describe('dynamic-href', () => {
const isDev = (global as any).isNextDev
if ((global as any).isNextDeploy) {
it('should skip next deploy for now', () => {})
return
}

let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: new FileRef(path.join(__dirname, 'dynamic-href')),
dependencies: {
react: 'experimental',
'react-dom': 'experimental',
},
})
})
afterAll(() => next.destroy())

if (isDev) {
it('should error when using dynamic href.pathname in app dir', async () => {
const browser = await webdriver(next.url, '/object')

// Error should show up
expect(await hasRedbox(browser, true)).toBeTrue()
expect(await getRedboxDescription(browser)).toMatchInlineSnapshot(
`"Error: Dynamic href \`/object/[slug]\` found in <Link> while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"`
)

// Fix error
const pageContent = await next.readFile('app/object/page.js')
await next.patchFile(
'app/object/page.js',
pageContent.replace(
"pathname: '/object/[slug]'",
"pathname: '/object/slug'"
)
)
expect(await browser.waitForElementByCss('#link').text()).toBe('to slug')

// Navigate to new page
await browser.elementByCss('#link').click()
expect(await browser.waitForElementByCss('#pathname').text()).toBe(
'/object/slug'
)
expect(await browser.elementByCss('#slug').text()).toBe('1')
})

it('should error when using dynamic href in app dir', async () => {
const browser = await webdriver(next.url, '/string')

// Error should show up
expect(await hasRedbox(browser, true)).toBeTrue()
expect(await getRedboxDescription(browser)).toMatchInlineSnapshot(
`"Error: Dynamic href \`/object/[slug]\` found in <Link> while using the \`/app\` router, this is not supported. Read more: https://nextjs.org/docs/messages/app-dir-dynamic-href"`
)
})
} else {
it('should not error on /object in prod', async () => {
const browser = await webdriver(next.url, '/object')
expect(await browser.elementByCss('#link').text()).toBe('to slug')
})
it('should not error on /string in prod', async () => {
const browser = await webdriver(next.url, '/string')
expect(await browser.elementByCss('#link').text()).toBe('to slug')
})
}
})
10 changes: 10 additions & 0 deletions test/e2e/app-dir/dynamic-href/app/layout.js
@@ -0,0 +1,10 @@
export default function Root({ children }) {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>{children}</body>
</html>
)
}
14 changes: 14 additions & 0 deletions test/e2e/app-dir/dynamic-href/app/object/[slug]/page.js
@@ -0,0 +1,14 @@
'use client'
import { usePathname, useSearchParams } from 'next/navigation'

export default function Page() {
const pathname = usePathname()
const searchParams = useSearchParams()

return (
<>
<p id="pathname">{pathname}</p>
<p id="slug">{searchParams.get('slug')}</p>
</>
)
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/dynamic-href/app/object/page.js
@@ -0,0 +1,15 @@
import Link from 'next/link'

export default function HomePage() {
return (
<Link
id="link"
href={{
pathname: '/object/[slug]',
query: { slug: '1' },
}}
>
to slug
</Link>
)
}
9 changes: 9 additions & 0 deletions test/e2e/app-dir/dynamic-href/app/string/page.js
@@ -0,0 +1,9 @@
import Link from 'next/link'

export default function HomePage() {
return (
<Link id="link" href="/object/[slug]">
to slug
</Link>
)
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/dynamic-href/next.config.js
@@ -0,0 +1,5 @@
module.exports = {
experimental: {
appDir: true,
},
}

0 comments on commit 0f195f0

Please sign in to comment.