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 7 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
30 changes: 30 additions & 0 deletions errors/app-dir-href-interpolation.md
@@ -0,0 +1,30 @@
# `href` interpolation is not supported in the `/app` directory

#### Why This Error Occurred

You have tried to use `href` interpolation with `next/link` in the `/app` directory. This is not supported.

#### Possible Ways to Fix It

**Before**

```jsx
<Link
href={{
pathname: '/route/[slug]',
query: { 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 @@ -761,6 +761,10 @@
{
"title": "next-router-not-mounted",
"path": "/errors/next-router-not-mounted.md"
},
{
"title": "app-dir-href-interpolation",
"path": "/errors/app-dir-href-interpolation.md"
}
]
}
Expand Down
26 changes: 26 additions & 0 deletions packages/next/client/link.tsx
Expand Up @@ -18,6 +18,7 @@ import {
import { useIntersection } from './use-intersection'
import { getDomainLocale } from './get-domain-locale'
import { addBasePath } from './add-base-path'
import { getSegmentParam } from '../server/utils'
hanneslund marked this conversation as resolved.
Show resolved Hide resolved

type Url = string | UrlObject
type RequiredKeys<T> = {
Expand Down Expand Up @@ -397,6 +398,31 @@ 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 &&
typeof hrefProp === 'object' &&
typeof hrefProp.pathname === 'string' &&
hrefProp.query &&
typeof hrefProp.query === 'object'
) {
const { pathname, query } = hrefProp

const dynamicSegments = pathname
.split('/')
.map((segment: string) => getSegmentParam(segment)?.param)
.filter(Boolean) as string[]

for (const dynamicSegment of dynamicSegments) {
if (query[dynamicSegment]) {
throw new Error(
`The \`/app\` router does not support \`href\` interpolation, found pathname \`${pathname}\` using query \`${dynamicSegment}\`. Read more: https://nextjs.org/docs/messages/app-dir-href-interpolation`
)
}
}
}
}

const { href, as } = React.useMemo(() => {
if (!pagesRouter) {
const resolvedHref = formatStringOrUrl(hrefProp)
Expand Down
33 changes: 1 addition & 32 deletions packages/next/server/app-render.tsx
Expand Up @@ -44,6 +44,7 @@ import {
RSC,
} from '../client/components/app-router-headers'
import type { StaticGenerationStore } from '../client/components/static-generation-async-storage'
import { type DynamicParamTypes, getSegmentParam } from './utils'

const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge'

Expand Down Expand Up @@ -437,7 +438,6 @@ function createServerComponentRenderer(
}
}

type DynamicParamTypes = 'catchall' | 'optional-catchall' | 'dynamic'
// c = catchall
// oc = optional catchall
// d = dynamic
Expand Down Expand Up @@ -533,37 +533,6 @@ export type ChildProp = {
segment: Segment
}

/**
* Parse dynamic route segment to type of parameter
*/
function getSegmentParam(segment: string): {
param: string
type: DynamicParamTypes
} | null {
if (segment.startsWith('[[...') && segment.endsWith(']]')) {
return {
type: 'optional-catchall',
param: segment.slice(5, -2),
}
}

if (segment.startsWith('[...') && segment.endsWith(']')) {
return {
type: 'catchall',
param: segment.slice(4, -1),
}
}

if (segment.startsWith('[') && segment.endsWith(']')) {
return {
type: 'dynamic',
param: segment.slice(1, -1),
}
}

return null
}

/**
* Get inline <link> tags based on server CSS manifest. Only used when rendering to HTML.
*/
Expand Down
32 changes: 32 additions & 0 deletions packages/next/server/utils.ts
Expand Up @@ -14,3 +14,35 @@ export function cleanAmpPath(pathname: string): string {
pathname = pathname.replace(/\?$/, '')
return pathname
}

export type DynamicParamTypes = 'catchall' | 'optional-catchall' | 'dynamic'
/**
* Parse dynamic route segment to type of parameter
*/
export function getSegmentParam(segment: string): {
param: string
type: DynamicParamTypes
} | null {
if (segment.startsWith('[[...') && segment.endsWith(']]')) {
return {
type: 'optional-catchall',
param: segment.slice(5, -2),
}
}

if (segment.startsWith('[...') && segment.endsWith(']')) {
return {
type: 'catchall',
param: segment.slice(4, -1),
}
}

if (segment.startsWith('[') && segment.endsWith(']')) {
return {
type: 'dynamic',
param: segment.slice(1, -1),
}
}

return null
}
58 changes: 58 additions & 0 deletions test/e2e/app-dir/href-interpolation.test.ts
@@ -0,0 +1,58 @@
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('href-interpolation', () => {
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, 'href-interpolation')),
dependencies: {
react: 'experimental',
'react-dom': 'experimental',
},
})
})
afterAll(() => next.destroy())

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

// Error should show up
expect(await hasRedbox(browser, true)).toBeTrue()
expect(await getRedboxDescription(browser)).toMatchInlineSnapshot(
`"Error: The \`/app\` router does not support \`href\` interpolation, found pathname \`/[slug]\` using query \`slug\`. Read more: https://nextjs.org/docs/messages/app-dir-href-interpolation"`
)

// Fix error
const pageContent = await next.readFile('app/page.js')
await next.patchFile(
'app/page.js',
pageContent.replace("pathname: '/[slug]'", "pathname: '/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(
'/slug'
)
expect(await browser.elementByCss('#slug').text()).toBe('1')
})
} else {
it('should not error in prod', async () => {
const browser = await webdriver(next.url, '/')
expect(await browser.elementByCss('#link').text()).toBe('to slug')
})
}
})
14 changes: 14 additions & 0 deletions test/e2e/app-dir/href-interpolation/app/[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>
</>
)
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/href-interpolation/app/layout.js
@@ -0,0 +1,10 @@
export default function Root({ children }) {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>{children}</body>
</html>
)
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/href-interpolation/app/page.js
@@ -0,0 +1,15 @@
import Link from 'next/link'

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