From a2f9ef5a34418d562581f54353ed51491a019484 Mon Sep 17 00:00:00 2001 From: Boris Besemer Date: Mon, 2 Oct 2023 23:43:04 +0200 Subject: [PATCH] fix(next/client): keep hash when navigating from app to pages router (#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 Co-authored-by: JJ Kasper --- .../router-reducer/fetch-server-response.ts | 5 +++++ .../app/hash-link-to-pages-router/global.css | 7 +++++++ .../app/hash-link-to-pages-router/page.js | 13 +++++++++++++ test/e2e/app-dir/navigation/navigation.test.ts | 9 +++++++++ 4 files changed, 34 insertions(+) create mode 100644 test/e2e/app-dir/navigation/app/hash-link-to-pages-router/global.css create mode 100644 test/e2e/app-dir/navigation/app/hash-link-to-pages-router/page.js diff --git a/packages/next/src/client/components/router-reducer/fetch-server-response.ts b/packages/next/src/client/components/router-reducer/fetch-server-response.ts index 38dcc1a740bd3..691f1be2f16c8 100644 --- a/packages/next/src/client/components/router-reducer/fetch-server-response.ts +++ b/packages/next/src/client/components/router-reducer/fetch-server-response.ts @@ -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()) } diff --git a/test/e2e/app-dir/navigation/app/hash-link-to-pages-router/global.css b/test/e2e/app-dir/navigation/app/hash-link-to-pages-router/global.css new file mode 100644 index 0000000000000..5d16eef0fe167 --- /dev/null +++ b/test/e2e/app-dir/navigation/app/hash-link-to-pages-router/global.css @@ -0,0 +1,7 @@ +* { + margin: 0; + padding: 0; + box-sizing: border-box; + font-size: 14px; + line-height: 1; +} diff --git a/test/e2e/app-dir/navigation/app/hash-link-to-pages-router/page.js b/test/e2e/app-dir/navigation/app/hash-link-to-pages-router/page.js new file mode 100644 index 0000000000000..cd026d9e5fa97 --- /dev/null +++ b/test/e2e/app-dir/navigation/app/hash-link-to-pages-router/page.js @@ -0,0 +1,13 @@ +import Link from 'next/link' +import './global.css' + +export default function HashPage() { + return ( +
+

Hash To Pages Router Page

+ + To pages router + +
+ ) +} diff --git a/test/e2e/app-dir/navigation/navigation.test.ts b/test/e2e/app-dir/navigation/navigation.test.ts index b3e1cf19c4261..e0559341bff1f 100644 --- a/test/e2e/app-dir/navigation/navigation.test.ts +++ b/test/e2e/app-dir/navigation/navigation.test.ts @@ -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