Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App directory next/link dynamic href dev error #43074

Merged
merged 23 commits into from Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.
hanneslund marked this conversation as resolved.
Show resolved Hide resolved

#### 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,
},
}