From 489cad36bcc95f93ce012712369a83809e91956d Mon Sep 17 00:00:00 2001
From: JJ Kasper <jj@jjsweb.site>
Date: Sat, 5 Sep 2020 21:36:57 -0500
Subject: [PATCH] Fix href resolving with trailing slash enabled (#16873)

This makes sure to strip the trailing slash before attempting to resolve the `href` against pages/dynamic routes and adds tests ensuring the correct pages are resolved with `trailingSlash: true` enabled.

Fixes: https://github.com/vercel/next.js/issues/16872
---
 .../next/next-server/lib/router/router.ts     |  4 +-
 .../next.config.js                            |  3 +
 .../pages/404.js                              |  3 +
 .../pages/[slug].js                           | 11 +++
 .../pages/another.js                          |  7 ++
 .../pages/blog/[slug].js                      | 11 +++
 .../pages/blog/another.js                     |  7 ++
 .../pages/catch-all/[...slug].js              | 11 +++
 .../pages/catch-all/first.js                  |  7 ++
 .../pages/index.js                            | 32 ++++++
 .../test/index.test.js                        | 98 +++++++++++++++++++
 11 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 test/integration/trailing-slashes-href-resolving/next.config.js
 create mode 100644 test/integration/trailing-slashes-href-resolving/pages/404.js
 create mode 100644 test/integration/trailing-slashes-href-resolving/pages/[slug].js
 create mode 100644 test/integration/trailing-slashes-href-resolving/pages/another.js
 create mode 100644 test/integration/trailing-slashes-href-resolving/pages/blog/[slug].js
 create mode 100644 test/integration/trailing-slashes-href-resolving/pages/blog/another.js
 create mode 100644 test/integration/trailing-slashes-href-resolving/pages/catch-all/[...slug].js
 create mode 100644 test/integration/trailing-slashes-href-resolving/pages/catch-all/first.js
 create mode 100644 test/integration/trailing-slashes-href-resolving/pages/index.js
 create mode 100644 test/integration/trailing-slashes-href-resolving/test/index.test.js

diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts
index 2782c5cb31..7d6f93d47d 100644
--- a/packages/next/next-server/lib/router/router.ts
+++ b/packages/next/next-server/lib/router/router.ts
@@ -922,7 +922,9 @@ export default class Router implements BaseRouter {
 
   _resolveHref(parsedHref: UrlObject, pages: string[]) {
     const { pathname } = parsedHref
-    const cleanPathname = denormalizePagePath(delBasePath(pathname!))
+    const cleanPathname = removePathTrailingSlash(
+      denormalizePagePath(delBasePath(pathname!))
+    )
 
     if (cleanPathname === '/404' || cleanPathname === '/_error') {
       return parsedHref
diff --git a/test/integration/trailing-slashes-href-resolving/next.config.js b/test/integration/trailing-slashes-href-resolving/next.config.js
new file mode 100644
index 0000000000..ce3f975d0e
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/next.config.js
@@ -0,0 +1,3 @@
+module.exports = {
+  trailingSlash: true,
+}
diff --git a/test/integration/trailing-slashes-href-resolving/pages/404.js b/test/integration/trailing-slashes-href-resolving/pages/404.js
new file mode 100644
index 0000000000..e16cd81e3b
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/pages/404.js
@@ -0,0 +1,3 @@
+export default function NotFound() {
+  return <div id="page-404">404</div>
+}
diff --git a/test/integration/trailing-slashes-href-resolving/pages/[slug].js b/test/integration/trailing-slashes-href-resolving/pages/[slug].js
new file mode 100644
index 0000000000..f873b7ab91
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/pages/[slug].js
@@ -0,0 +1,11 @@
+import { useRouter } from 'next/router'
+
+export default function Page() {
+  const router = useRouter()
+
+  return (
+    <>
+      <p id="slug">top level slug {router.query.slug}</p>
+    </>
+  )
+}
diff --git a/test/integration/trailing-slashes-href-resolving/pages/another.js b/test/integration/trailing-slashes-href-resolving/pages/another.js
new file mode 100644
index 0000000000..decf00835a
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/pages/another.js
@@ -0,0 +1,7 @@
+export default function Page() {
+  return (
+    <>
+      <p id="another">top level another</p>
+    </>
+  )
+}
diff --git a/test/integration/trailing-slashes-href-resolving/pages/blog/[slug].js b/test/integration/trailing-slashes-href-resolving/pages/blog/[slug].js
new file mode 100644
index 0000000000..f67b5c66bf
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/pages/blog/[slug].js
@@ -0,0 +1,11 @@
+import { useRouter } from 'next/router'
+
+export default function Page() {
+  const router = useRouter()
+
+  return (
+    <>
+      <p id="slug">blog slug {router.query.slug}</p>
+    </>
+  )
+}
diff --git a/test/integration/trailing-slashes-href-resolving/pages/blog/another.js b/test/integration/trailing-slashes-href-resolving/pages/blog/another.js
new file mode 100644
index 0000000000..04416504c5
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/pages/blog/another.js
@@ -0,0 +1,7 @@
+export default function Page() {
+  return (
+    <>
+      <p id="another">blog another</p>
+    </>
+  )
+}
diff --git a/test/integration/trailing-slashes-href-resolving/pages/catch-all/[...slug].js b/test/integration/trailing-slashes-href-resolving/pages/catch-all/[...slug].js
new file mode 100644
index 0000000000..328deb70ac
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/pages/catch-all/[...slug].js
@@ -0,0 +1,11 @@
+import { useRouter } from 'next/router'
+
+export default function Page() {
+  const router = useRouter()
+
+  return (
+    <>
+      <p id="slug">catch-all slug {router.query.slug?.join('/')}</p>
+    </>
+  )
+}
diff --git a/test/integration/trailing-slashes-href-resolving/pages/catch-all/first.js b/test/integration/trailing-slashes-href-resolving/pages/catch-all/first.js
new file mode 100644
index 0000000000..ef828deb44
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/pages/catch-all/first.js
@@ -0,0 +1,7 @@
+export default function Page() {
+  return (
+    <>
+      <p id="first">catch-all first</p>
+    </>
+  )
+}
diff --git a/test/integration/trailing-slashes-href-resolving/pages/index.js b/test/integration/trailing-slashes-href-resolving/pages/index.js
new file mode 100644
index 0000000000..46e6e09936
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/pages/index.js
@@ -0,0 +1,32 @@
+import Link from 'next/link'
+
+export default function Index() {
+  return (
+    <>
+      <Link href="/blog/another/">
+        <a id="to-blog-another">to /blog/another/</a>
+      </Link>
+      <br />
+      <Link href="/blog/first-post/">
+        <a id="to-blog-post">to /blog/first-post/</a>
+      </Link>
+      <br />
+      <Link href="/catch-all/hello/world/">
+        <a id="to-catch-all-item">to /catch-all/hello/world/</a>
+      </Link>
+      <br />
+      <Link href="/catch-all/first/">
+        <a id="to-catch-all-first">to /catch-all/first/</a>
+      </Link>
+      <br />
+      <Link href="/another/">
+        <a id="to-another">to /another/</a>
+      </Link>
+      <br />
+      <Link href="/top-level-slug/">
+        <a id="to-slug">to /top-level-slug/</a>
+      </Link>
+      <br />
+    </>
+  )
+}
diff --git a/test/integration/trailing-slashes-href-resolving/test/index.test.js b/test/integration/trailing-slashes-href-resolving/test/index.test.js
new file mode 100644
index 0000000000..eb4811538b
--- /dev/null
+++ b/test/integration/trailing-slashes-href-resolving/test/index.test.js
@@ -0,0 +1,98 @@
+/* eslint-env jest */
+
+import { join } from 'path'
+import webdriver from 'next-webdriver'
+import {
+  findPort,
+  killApp,
+  launchApp,
+  nextBuild,
+  nextStart,
+} from 'next-test-utils'
+
+jest.setTimeout(1000 * 60 * 2)
+
+let app
+let appPort
+const appDir = join(__dirname, '../')
+
+const runTests = () => {
+  it('should route to /blog/another/ correctly', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.elementByCss('#to-blog-another').click()
+
+    await browser.waitForElementByCss('#another')
+    expect(await browser.elementByCss('#another').text()).toBe('blog another')
+  })
+
+  it('should route to /blog/first-post/ correctly', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.elementByCss('#to-blog-post').click()
+
+    await browser.waitForElementByCss('#slug')
+    expect(await browser.elementByCss('#slug').text()).toBe(
+      'blog slug first-post'
+    )
+  })
+
+  it('should route to /catch-all/hello/world/ correctly', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.elementByCss('#to-catch-all-item').click()
+
+    await browser.waitForElementByCss('#slug')
+    expect(await browser.elementByCss('#slug').text()).toBe(
+      'catch-all slug hello/world'
+    )
+  })
+
+  it('should route to /catch-all/first/ correctly', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.elementByCss('#to-catch-all-first').click()
+
+    await browser.waitForElementByCss('#first')
+    expect(await browser.elementByCss('#first').text()).toBe('catch-all first')
+  })
+
+  it('should route to /another/ correctly', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.elementByCss('#to-another').click()
+
+    await browser.waitForElementByCss('#another')
+    expect(await browser.elementByCss('#another').text()).toBe(
+      'top level another'
+    )
+  })
+
+  it('should route to /top-level-slug/ correctly', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.elementByCss('#to-slug').click()
+
+    await browser.waitForElementByCss('#slug')
+    expect(await browser.elementByCss('#slug').text()).toBe(
+      'top level slug top-level-slug'
+    )
+  })
+}
+
+describe('href resolving trailing-slash', () => {
+  describe('dev mode', () => {
+    beforeAll(async () => {
+      appPort = await findPort()
+      app = await launchApp(appDir, appPort)
+    })
+    afterAll(() => killApp(app))
+
+    runTests()
+  })
+
+  describe('production mode', () => {
+    beforeAll(async () => {
+      await nextBuild(appDir)
+      appPort = await findPort()
+      app = await nextStart(appDir, appPort)
+    })
+    afterAll(() => killApp(app))
+
+    runTests()
+  })
+})
-- 
GitLab