Skip to content

Commit

Permalink
fix(next/client): keep hash when navigating from app to pages router (#…
Browse files Browse the repository at this point in the history
…56223)

### What?

Fixes the pages router not receiving a hash when being linked from the
app router.

### Why?

The hash being removed breaks sites that rely on it for client side
features.

### How?

The hash gets omitted from the URL when used as a key for the
preflightCache. Once it's clear that it's an external URL and that it's
not empty we can use the initial href to send the hash as well.

Not completely sure if there's an edge case that might break, I added an
extra check for when the hash is only used to scroll the page.

This might need an additional test case just for
`navigate-reducer.test.tsx`.

Fixes #56212

---------

Co-authored-by: Zack Tanner <zacktanner@gmail.com>
Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
3 people authored Oct 2, 2023
1 parent a970f28 commit a2f9ef5
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ export async function fetchServerResponse(
// If fetch returns something different than flight response handle it like a mpa navigation
// If the fetch was not 200, we also handle it like a mpa navigation
if (!isFlightResponse || !res.ok) {
// in case the original URL came with a hash, preserve it before redirecting to the new URL
if (url.hash) {
responseUrl.hash = url.hash
}

return doMpaNavigation(responseUrl.toString())
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
* {
margin: 0;
padding: 0;
box-sizing: border-box;
font-size: 14px;
line-height: 1;
}
13 changes: 13 additions & 0 deletions test/e2e/app-dir/navigation/app/hash-link-to-pages-router/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import Link from 'next/link'
import './global.css'

export default function HashPage() {
return (
<div style={{ fontFamily: 'sans-serif', fontSize: '16px' }}>
<p>Hash To Pages Router Page</p>
<Link href="/some#non-existent" id="link-to-pages-router">
To pages router
</Link>
</div>
)
}
9 changes: 9 additions & 0 deletions test/e2e/app-dir/navigation/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,15 @@ createNextDescribe(
expect(await browser.url()).toBe(next.url + '/some')
})

it('should not omit the hash while navigating from app to pages', async () => {
const browser = await next.browser('/hash-link-to-pages-router')
await browser
.elementByCss('#link-to-pages-router')
.click()
.waitForElementByCss('#link-to-app')
await check(() => browser.url(), next.url + '/some#non-existent')
})

if (!isNextDev) {
// this test is pretty hard to test in playwright, so most of the heavy lifting is in the page component itself
// it triggers a hover on a link to initiate a prefetch request every second, and so we check that
Expand Down

0 comments on commit a2f9ef5

Please sign in to comment.