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>
vercel · Feb 7, 2022 · b49717f · b49717f
1 parent 41706d6
commit b49717f
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 {