From b49717f8dbf530f2fe8a2af8c8c3f7a3ae1209d5 Mon Sep 17 00:00:00 2001
From: Wouter Raateland <wouterraateland@gmail.com>
Date: Mon, 7 Feb 2022 01:38:55 +0100
Subject: [PATCH] Allow scroll prevention on hash change (#31921)

* Allow scroll prevention on hash change

Currently, `scrollToHash` is performed on every hash change, even when this change is caused by `<Link scroll={false} {...props} />`.
This change prevents scrolling in this case and allows users to specify the desired scrolling behavior in the router's `hashChangeComplete` event.

* Add test case and apply fixes

Co-authored-by: JJ Kasper <jj@jjsweb.site>
---
 packages/next/client/link.tsx                         |  5 -----
 packages/next/shared/lib/router/router.ts             | 11 ++++++++---
 .../client-navigation/pages/nav/hash-changes.js       |  5 +++++
 test/integration/client-navigation/test/index.test.js | 11 +++++++++++
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx
index 9a7f65b52997b..9bb634d251adc 100644
--- a/packages/next/client/link.tsx
+++ b/packages/next/client/link.tsx
@@ -96,11 +96,6 @@ function linkClicked(
 
   e.preventDefault()
 
-  //  avoid scroll for urls with anchor refs
-  if (scroll == null && as.indexOf('#') >= 0) {
-    scroll = false
-  }
-
   // replace state instead of push if prop is present
   router[replace ? 'replace' : 'push'](href, as, {
     shallow,
diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts
index 9a41567ba0bfa..ed80980691fd1 100644
--- a/packages/next/shared/lib/router/router.ts
+++ b/packages/next/shared/lib/router/router.ts
@@ -1009,7 +1009,7 @@ export default class Router implements BaseRouter {
       performance.mark('routeChange')
     }
 
-    const { shallow = false } = options
+    const { shallow = false, scroll = true } = options
     const routeProps = { shallow }
 
     if (this._inFlightRoute) {
@@ -1045,8 +1045,13 @@ export default class Router implements BaseRouter {
       this.asPath = cleanedAs
       Router.events.emit('hashChangeStart', as, routeProps)
       // TODO: do we need the resolved href when only a hash change?
-      this.changeState(method, url, as, options)
-      this.scrollToHash(cleanedAs)
+      this.changeState(method, url, as, {
+        ...options,
+        scroll: false,
+      })
+      if (scroll) {
+        this.scrollToHash(cleanedAs)
+      }
       this.notify(this.components[this.route], null)
       Router.events.emit('hashChangeComplete', as, routeProps)
       return true
diff --git a/test/integration/client-navigation/pages/nav/hash-changes.js b/test/integration/client-navigation/pages/nav/hash-changes.js
index e76f3a22d89fc..67c9a35cde53f 100644
--- a/test/integration/client-navigation/pages/nav/hash-changes.js
+++ b/test/integration/client-navigation/pages/nav/hash-changes.js
@@ -27,6 +27,11 @@ const HashChanges = ({ count }) => {
       <Link href="#name-item-400">
         <a id="scroll-to-name-item-400">Go to name item 400</a>
       </Link>
+      <Link href="#name-item-400" scroll={false}>
+        <a id="scroll-to-name-item-400-no-scroll">
+          Go to name item 400 (no scroll)
+        </a>
+      </Link>
       <p>COUNT: {count}</p>
       {Array.from({ length: 500 }, (x, i) => i + 1).map((i) => {
         return (
diff --git a/test/integration/client-navigation/test/index.test.js b/test/integration/client-navigation/test/index.test.js
index 4a6921dfdd446..f89aeed941ee7 100644
--- a/test/integration/client-navigation/test/index.test.js
+++ b/test/integration/client-navigation/test/index.test.js
@@ -625,6 +625,17 @@ describe('Client Navigation', () => {
         }
       })
 
+      it('should not scroll to hash when scroll={false} is set', async () => {
+        const browser = await webdriver(context.appPort, '/nav/hash-changes')
+        const curScroll = await browser.eval(
+          'document.documentElement.scrollTop'
+        )
+        await browser.elementByCss('#scroll-to-name-item-400-no-scroll').click()
+        expect(curScroll).toBe(
+          await browser.eval('document.documentElement.scrollTop')
+        )
+      })
+
       it('should scroll to the specified position on the same page with a name property', async () => {
         let browser
         try {