Skip to content

Commit

Permalink
App Router: fix relative query/hash handling in next/link and route…
Browse files Browse the repository at this point in the history
…r push/replace (vercel#49521)

## Problem
Relative hash/query handling in `next/link` (e.g. `<Link
href="#hello">`) is broken in App Router, especially if you're on a
nested route.

This wasn't a problem in `/pages` because the href always get fully
resolved in `<NextLink>`; i.e. if you have `<Link href="#hash" />` on
`/hello`, it'll resolve the href to `/hello#hash` and use that
everywhere.

However, in App Router, `<Link>` no longer uses the current location to
resolve the href:
https://github.com/vercel/next.js/blob/5451564f364399003b05939fa6dd7d32c1dabec7/packages/next/src/client/link.tsx#L450-L457

Therefore navigating with `new URL(href, location.origin)` will skip the
current path and always apply the relative hash/query to the _root_
route:
https://github.com/vercel/next.js/blob/5451564f364399003b05939fa6dd7d32c1dabec7/packages/next/src/client/components/app-router.tsx#L208-L215

## Solution
Not 100% sure if this is the best solution, but since `app-router` is
already reading `window.location`, I'm using `location.href` as the base
URL to resolve the href.

I grep'd for `location.origin` and checked other callsites; seems like
only `app-router` deals with user specified hrefs.

Fixes vercel#42157 &
vercel#44139, and potentially
vercel#48554 and
vercel#22838

## Test Plan
```
pnpm testheadless test/e2e/app-dir/navigation
```

---------
  • Loading branch information
keyz authored and hydRAnger committed Jun 12, 2023
1 parent 8a62462 commit 841dacf
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ function Router({
navigateType: 'push' | 'replace',
forceOptimisticNavigation: boolean
) => {
const url = new URL(addBasePath(href), location.origin)
const url = new URL(addBasePath(href), location.href)

return dispatch({
type: ACTION_NAVIGATE,
Expand Down Expand Up @@ -261,7 +261,7 @@ function Router({
if (isBot(window.navigator.userAgent)) {
return
}
const url = new URL(addBasePath(href), location.origin)
const url = new URL(addBasePath(href), location.href)
// External urls can't be prefetched in the same way.
if (isExternalURL(url)) {
return
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/navigation/app/hash/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ export default function HashPage() {
<Link href="/hash#hash-300" id="link-to-300">
To 300
</Link>
<Link href="#hash-500" id="link-to-500">
To 500 (hash only)
</Link>
<Link href="/hash#top" id="link-to-top">
To Top
</Link>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use client'

import { useRouter } from 'next/navigation'

export function RouterPushButton() {
const router = useRouter()

return (
<h3 id="h3">
<button
id="button-to-h3-hash-only"
onClick={() => {
router.push('#h3')
}}
>
To #h3, hash only
</button>
</h3>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import Link from 'next/link'
import { RouterPushButton } from './client-component'

export default function Page() {
return (
<>
<h1 id="h1">
<Link href="#h1" id="link-to-h1-hash-only">
To #h1, hash only
</Link>
</h1>

<p>
<Link href="?foo=1&bar=2" id="link-to-dummy-query">
Query only
</Link>
</p>

<h2 id="h2">
<Link href="?here=ok#h2" id="link-to-h2-with-hash-and-query">
To #h2, with both relative hash and query
</Link>
</h2>

<RouterPushButton />
</>
)
}
55 changes: 55 additions & 0 deletions test/e2e/app-dir/navigation/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ createNextDescribe(
await checkLink(50, 730)
await checkLink(160, 2270)
await checkLink(300, 4230)
await checkLink(500, 7030) // this one is hash only (`href="#hash-500"`)
await checkLink('top', 0)
await checkLink('non-existent', 0)
})
Expand Down Expand Up @@ -106,6 +107,60 @@ createNextDescribe(
})
})

describe('relative hashes and queries', () => {
const pathname = '/nested-relative-query-and-hash'

it('should work with a hash-only href', async () => {
const browser = await next.browser(pathname)
await browser.elementByCss('#link-to-h1-hash-only').click()

await check(() => browser.url(), next.url + pathname + '#h1')
})

it('should work with a hash-only `router.push(...)`', async () => {
const browser = await next.browser(pathname)
await browser.elementByCss('#button-to-h3-hash-only').click()

await check(() => browser.url(), next.url + pathname + '#h3')
})

it('should work with a query-only href', async () => {
const browser = await next.browser(pathname)
await browser.elementByCss('#link-to-dummy-query').click()

await check(() => browser.url(), next.url + pathname + '?foo=1&bar=2')
})

it('should work with both relative hashes and queries', async () => {
const browser = await next.browser(pathname)
await browser.elementByCss('#link-to-h2-with-hash-and-query').click()

await check(() => browser.url(), next.url + pathname + '?here=ok#h2')

// Only update hash
await browser.elementByCss('#link-to-h1-hash-only').click()
await check(() => browser.url(), next.url + pathname + '?here=ok#h1')

// Replace all with new query
await browser.elementByCss('#link-to-dummy-query').click()
await check(() => browser.url(), next.url + pathname + '?foo=1&bar=2')

// Add hash to existing query
await browser.elementByCss('#link-to-h1-hash-only').click()
await check(
() => browser.url(),
next.url + pathname + '?foo=1&bar=2#h1'
)

// Update hash again via `router.push(...)`
await browser.elementByCss('#button-to-h3-hash-only').click()
await check(
() => browser.url(),
next.url + pathname + '?foo=1&bar=2#h3'
)
})
})

describe('not-found', () => {
it('should trigger not-found in a server component', async () => {
const browser = await next.browser('/not-found/servercomponent')
Expand Down

0 comments on commit 841dacf

Please sign in to comment.