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: #16872

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

test/integration/trailing-slashes-href-resolving/next.config.js

@@ -0,0 +1,3 @@
+module.exports = {
+  trailingSlash: true,
+}

test/integration/trailing-slashes-href-resolving/pages/404.js

@@ -0,0 +1,3 @@
+export default function NotFound() {
+  return <div id="page-404">404</div>
+}

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>
+    </>
+  )
+}

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>
+    </>
+  )
+}

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>
+    </>
+  )
+}

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>
+    </>
+  )
+}

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>
+    </>
+  )
+}

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>
+    </>
+  )
+}

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 />
+    </>
+  )
+}

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()
+  })
+})