From 0f195f0270c0be1099bca5a8bbf870b5194d6b08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20Born=C3=B6?= <hannes.lund@gmail.com>
Date: Fri, 25 Nov 2022 15:03:00 +0100
Subject: [PATCH] App directory next/link dynamic href dev error (#43074)

Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
Fixes https://github.com/vercel/next.js/issues/42715
---
 errors/app-dir-dynamic-href.md                | 36 +++++++++
 errors/manifest.json                          |  4 +
 packages/next/client/link.tsx                 | 26 +++++++
 test/e2e/app-dir/dynamic-href.test.ts         | 75 +++++++++++++++++++
 test/e2e/app-dir/dynamic-href/app/layout.js   | 10 +++
 .../dynamic-href/app/object/[slug]/page.js    | 14 ++++
 .../app-dir/dynamic-href/app/object/page.js   | 15 ++++
 .../app-dir/dynamic-href/app/string/page.js   |  9 +++
 test/e2e/app-dir/dynamic-href/next.config.js  |  5 ++
 9 files changed, 194 insertions(+)
 create mode 100644 errors/app-dir-dynamic-href.md
 create mode 100644 test/e2e/app-dir/dynamic-href.test.ts
 create mode 100644 test/e2e/app-dir/dynamic-href/app/layout.js
 create mode 100644 test/e2e/app-dir/dynamic-href/app/object/[slug]/page.js
 create mode 100644 test/e2e/app-dir/dynamic-href/app/object/page.js
 create mode 100644 test/e2e/app-dir/dynamic-href/app/string/page.js
 create mode 100644 test/e2e/app-dir/dynamic-href/next.config.js

diff --git a/errors/app-dir-dynamic-href.md b/errors/app-dir-dynamic-href.md
new file mode 100644
index 000000000000..88c76c5319c7
--- /dev/null
+++ b/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)
diff --git a/errors/manifest.json b/errors/manifest.json
index 0d4f445731ca..21e8f8478245 100644
--- a/errors/manifest.json
+++ b/errors/manifest.json
@@ -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"
         }
       ]
     }
diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx
index 54b51d8a51bb..6da55598e464 100644
--- a/packages/next/client/link.tsx
+++ b/packages/next/client/link.tsx
@@ -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)
diff --git a/test/e2e/app-dir/dynamic-href.test.ts b/test/e2e/app-dir/dynamic-href.test.ts
new file mode 100644
index 000000000000..6615538c55e2
--- /dev/null
+++ b/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')
+    })
+  }
+})
diff --git a/test/e2e/app-dir/dynamic-href/app/layout.js b/test/e2e/app-dir/dynamic-href/app/layout.js
new file mode 100644
index 000000000000..c84b681925eb
--- /dev/null
+++ b/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>
+  )
+}
diff --git a/test/e2e/app-dir/dynamic-href/app/object/[slug]/page.js b/test/e2e/app-dir/dynamic-href/app/object/[slug]/page.js
new file mode 100644
index 000000000000..911892225aff
--- /dev/null
+++ b/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>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/dynamic-href/app/object/page.js b/test/e2e/app-dir/dynamic-href/app/object/page.js
new file mode 100644
index 000000000000..bcc1956d7e73
--- /dev/null
+++ b/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>
+  )
+}
diff --git a/test/e2e/app-dir/dynamic-href/app/string/page.js b/test/e2e/app-dir/dynamic-href/app/string/page.js
new file mode 100644
index 000000000000..a3349d59e24e
--- /dev/null
+++ b/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>
+  )
+}
diff --git a/test/e2e/app-dir/dynamic-href/next.config.js b/test/e2e/app-dir/dynamic-href/next.config.js
new file mode 100644
index 000000000000..cfa3ac3d7aa9
--- /dev/null
+++ b/test/e2e/app-dir/dynamic-href/next.config.js
@@ -0,0 +1,5 @@
+module.exports = {
+  experimental: {
+    appDir: true,
+  },
+}