next.js icon indicating copy to clipboard operation
next.js copied to clipboard

Published 20 hours ago • verified publisher icon vercel

Fix scroll position being reset on back and forward navigation

Open shuding opened this issue 3 years ago • 6 comments

This PR fixes the behavior that Next.js always resets the scroll position to (0, 0) on popState, it's not noticeable because the browser corrects it after that. When explicitly set history.scrollRestoration to 'manual' we are able to reproduce it.

Also adds a test because the current ones only cover the scrollRestoration experimental flag.

Fixes #20951.

shuding avatar Mar 03 '21 13:03 shuding

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
buildDuration 11.5s 12s ⚠️ +425ms
nodeModulesSize 42.7 MB 42.7 MB ⚠️ +56 B
Page Load Tests Overall increase ✓
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.203 2.229 ⚠️ +0.03
/ avg req/sec 1134.6 1121.73 ⚠️ -12.87
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.579 1.506 -0.07
/error-in-render avg req/sec 1583.62 1660.17 +76.55
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
677f882d2ed8..e7a0.js gzip 13.4 kB 13.4 kB ⚠️ +5 B
framework.HASH.js gzip 39 kB 39 kB ✓
main-HASH.js gzip 6.65 kB 6.65 kB ✓
webpack-HASH.js gzip 751 B 751 B ✓
Overall change 59.7 kB 59.7 kB ⚠️ +5 B
Legacy Client Bundles (polyfills)
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
polyfills-HASH.js gzip 31.3 kB 31.3 kB ✓
Overall change 31.3 kB 31.3 kB ✓
Client Pages
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
_app-2a09aa2..4a98.js gzip 1.28 kB 1.28 kB ✓
_error-8b758..aef6.js gzip 3.46 kB 3.46 kB ✓
amp-e3e9bc99..932c.js gzip 536 B 536 B ✓
hooks-5023ef..3670.js gzip 888 B 888 B ✓
index-3a2b03..c677.js gzip 227 B 227 B ✓
link-177412b..043e.js gzip 1.67 kB 1.67 kB ✓
routerDirect..1a66.js gzip 303 B 303 B ✓
withRouter-9..1b7d.js gzip 302 B 302 B ✓
Overall change 8.66 kB 8.66 kB ✓
Client Build Manifests
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
_buildManifest.js gzip 347 B 347 B ✓
Overall change 347 B 347 B ✓
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
index.html gzip 613 B 614 B ⚠️ +1 B
link.html gzip 620 B 622 B ⚠️ +2 B
withRouter.html gzip 608 B 609 B ⚠️ +1 B
Overall change 1.84 kB 1.84 kB ⚠️ +4 B

Diffs

Diff for 677f882d2ed8..5ec6837e3.js 
@@ -1701,7 +1701,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
               as,
               Object.assign({}, options, {
                 shallow: options.shallow && _this._shallow,
-                locale: options.locale || _this.defaultLocale
+                locale: options.locale || _this.defaultLocale,
+                scroll: false
               }),
               forcedScroll
             );
Diff for index.html 
@@ -22,7 +22,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.42120b530b65ec6837e3.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2c018116a9c3de3f916f.js"
       as="script"
     />
     <link
@@ -65,7 +65,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.42120b530b65ec6837e3.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2c018116a9c3de3f916f.js"
       async=""
     ></script>
     <script
Diff for link.html 
@@ -22,7 +22,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.42120b530b65ec6837e3.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2c018116a9c3de3f916f.js"
       as="script"
     />
     <link
@@ -70,7 +70,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.42120b530b65ec6837e3.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2c018116a9c3de3f916f.js"
       async=""
     ></script>
     <script
Diff for withRouter.html 
@@ -22,7 +22,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.42120b530b65ec6837e3.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2c018116a9c3de3f916f.js"
       as="script"
     />
     <link
@@ -65,7 +65,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.42120b530b65ec6837e3.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2c018116a9c3de3f916f.js"
       async=""
     ></script>
     <script
Serverless Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
buildDuration 13.9s 14.1s ⚠️ +177ms
nodeModulesSize 42.7 MB 42.7 MB ⚠️ +56 B
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
677f882d2ed8..e7a0.js gzip 13.4 kB N/A N/A
framework.HASH.js gzip 39 kB 39 kB ✓
main-HASH.js gzip 6.65 kB 6.65 kB ✓
webpack-HASH.js gzip 751 B 751 B ✓
677f882d2ed8..94af.js gzip N/A 13.4 kB N/A
Overall change 59.7 kB 59.7 kB ⚠️ +5 B
Legacy Client Bundles (polyfills)
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
polyfills-HASH.js gzip 31.3 kB 31.3 kB ✓
Overall change 31.3 kB 31.3 kB ✓
Client Pages
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
_app-2a09aa2..4a98.js gzip 1.28 kB 1.28 kB ✓
_error-8b758..aef6.js gzip 3.46 kB 3.46 kB ✓
amp-e3e9bc99..932c.js gzip 536 B 536 B ✓
hooks-5023ef..3670.js gzip 888 B 888 B ✓
index-3a2b03..c677.js gzip 227 B 227 B ✓
link-177412b..043e.js gzip 1.67 kB 1.67 kB ✓
routerDirect..1a66.js gzip 303 B 303 B ✓
withRouter-9..1b7d.js gzip 302 B 302 B ✓
Overall change 8.66 kB 8.66 kB ✓
Client Build Manifests
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
_buildManifest.js gzip 347 B 347 B ✓
Overall change 347 B 347 B ✓
Serverless bundles Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
_error.js 1.02 MB 1.02 MB ✓
404.html 2.67 kB 2.67 kB ✓
500.html 2.65 kB 2.65 kB ✓
amp.amp.html 10.4 kB 10.4 kB ✓
amp.html 1.86 kB 1.86 kB ✓
hooks.html 1.92 kB 1.92 kB ✓
index.js 1.02 MB 1.02 MB ✓
link.js 1.08 MB 1.08 MB ⚠️ +23 B
routerDirect.js 1.07 MB 1.07 MB ⚠️ +23 B
withRouter.js 1.07 MB 1.07 MB ⚠️ +23 B
Overall change 5.27 MB 5.27 MB ⚠️ +69 B
Webpack 5 Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
buildDuration 14.3s 14.3s ⚠️ +42ms
nodeModulesSize 42.7 MB 42.7 MB ⚠️ +56 B
Page Load Tests Overall increase ✓
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.167 2.134 -0.03
/ avg req/sec 1153.73 1171.77 +18.04
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.583 1.587 0
/error-in-render avg req/sec 1578.79 1575.77 ⚠️ -3.02
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
597-74632e79..1d47.js gzip 13.3 kB 13.3 kB ⚠️ +4 B
framework.HASH.js gzip 39.3 kB 39.3 kB ✓
main-HASH.js gzip 6.6 kB 6.6 kB ✓
webpack-HASH.js gzip 954 B 954 B ✓
Overall change 60.1 kB 60.1 kB ⚠️ +4 B
Legacy Client Bundles (polyfills)
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB ✓
Overall change 31.1 kB 31.1 kB ✓
Client Pages
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
_app-aedc815..1421.js gzip 1.26 kB 1.26 kB ✓
_error-71ec2..1a96.js gzip 3.38 kB 3.38 kB ✓
amp-33a09cb0..6745.js gzip 536 B 536 B ✓
hooks-4e968a..f870.js gzip 902 B 902 B ✓
index-5c6845..f75c.js gzip 230 B 230 B ✓
link-99f0c6c..b84a.js gzip 1.65 kB 1.65 kB ✓
routerDirect..bb56.js gzip 306 B 306 B ✓
withRouter-7..2133.js gzip 302 B 302 B ✓
Overall change 8.57 kB 8.57 kB ✓
Client Build Manifests
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
_buildManifest.js gzip 326 B 326 B ✓
Overall change 326 B 326 B ✓
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary shuding/next.js fix-manual-scroll-restoration Change
index.html gzip 586 B 587 B ⚠️ +1 B
link.html gzip 593 B 592 B -1 B
withRouter.html gzip 581 B 582 B ⚠️ +1 B
Overall change 1.76 kB 1.76 kB ⚠️ +1 B

Diffs

Diff for 677f882d2ed8..eaa79e7a0.js 
@@ -969,7 +969,8 @@
                       n,
                       Object.assign({}, o, {
                         shallow: o.shallow && i._shallow,
-                        locale: o.locale || i.defaultLocale
+                        locale: o.locale || i.defaultLocale,
+                        scroll: !1
                       }),
                       undefined
                     );
Diff for index.html 
@@ -22,7 +22,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.45a1f5b1723eaa79e7a0.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a973e2eb466e7e9c94af.js"
       as="script"
     />
     <link
@@ -43,7 +43,7 @@
         "props": { "pageProps": {} },
         "page": "/",
         "query": {},
-        "buildId": "8zTew-6sWr0fSIt2oV0ym",
+        "buildId": "oDhfaeVkUGJg9FA98dyTH",
         "isFallback": false,
         "gip": true
       }
@@ -65,7 +65,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.45a1f5b1723eaa79e7a0.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a973e2eb466e7e9c94af.js"
       async=""
     ></script>
     <script
@@ -77,11 +77,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/8zTew-6sWr0fSIt2oV0ym/_buildManifest.js"
+      src="/_next/static/oDhfaeVkUGJg9FA98dyTH/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/8zTew-6sWr0fSIt2oV0ym/_ssgManifest.js"
+      src="/_next/static/oDhfaeVkUGJg9FA98dyTH/_ssgManifest.js"
       async=""
     ></script>
   </body>
Diff for link.html 
@@ -22,7 +22,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.45a1f5b1723eaa79e7a0.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a973e2eb466e7e9c94af.js"
       as="script"
     />
     <link
@@ -48,7 +48,7 @@
         "props": { "pageProps": {} },
         "page": "/link",
         "query": {},
-        "buildId": "8zTew-6sWr0fSIt2oV0ym",
+        "buildId": "oDhfaeVkUGJg9FA98dyTH",
         "isFallback": false,
         "gip": true
       }
@@ -70,7 +70,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.45a1f5b1723eaa79e7a0.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a973e2eb466e7e9c94af.js"
       async=""
     ></script>
     <script
@@ -82,11 +82,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/8zTew-6sWr0fSIt2oV0ym/_buildManifest.js"
+      src="/_next/static/oDhfaeVkUGJg9FA98dyTH/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/8zTew-6sWr0fSIt2oV0ym/_ssgManifest.js"
+      src="/_next/static/oDhfaeVkUGJg9FA98dyTH/_ssgManifest.js"
       async=""
     ></script>
   </body>
Diff for withRouter.html 
@@ -22,7 +22,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.45a1f5b1723eaa79e7a0.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a973e2eb466e7e9c94af.js"
       as="script"
     />
     <link
@@ -43,7 +43,7 @@
         "props": { "pageProps": {} },
         "page": "/withRouter",
         "query": {},
-        "buildId": "8zTew-6sWr0fSIt2oV0ym",
+        "buildId": "oDhfaeVkUGJg9FA98dyTH",
         "isFallback": false,
         "gip": true
       }
@@ -65,7 +65,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.45a1f5b1723eaa79e7a0.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a973e2eb466e7e9c94af.js"
       async=""
     ></script>
     <script
@@ -77,11 +77,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/8zTew-6sWr0fSIt2oV0ym/_buildManifest.js"
+      src="/_next/static/oDhfaeVkUGJg9FA98dyTH/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/8zTew-6sWr0fSIt2oV0ym/_ssgManifest.js"
+      src="/_next/static/oDhfaeVkUGJg9FA98dyTH/_ssgManifest.js"
       async=""
     ></script>
   </body>
Commit: 9fc81eaee487fddccfc613eb2f157d14f8e767f5

ijjk avatar Mar 03 '21 13:03 ijjk

Instead of just assigning to false at least this should be configurable from somewhere. That would be the desired use-case

bayraak avatar Mar 09 '21 16:03 bayraak

Hi @bayraak! The reason for directly using false here is, no matter what the value of history.scrollRestoration is, Next.js should never reset the scroll position to (0,0) when hitting back/forward buttons (also mentioned in the original issue comment by another maintainer: https://github.com/vercel/next.js/issues/20951#issuecomment-757941531). I can't think of any specific use case that you want to configure that.

The false change in this PR will only affect the navigations triggered by history's popstate event, which are only back/forward. Link navigations are still configurable via scroll={false} of <Link>.

shuding avatar Mar 09 '21 16:03 shuding

@shuding FYI, you can achieve the same behaviour by using the experimental scrollRestoration flag -- in your next.config.js, add:

module.exports = {
  experimental: {
    scrollRestoration: true
  }
}

Related PR: https://github.com/vercel/next.js/commit/38bd1a024cb25923d8ea15f269a7294d073684d8

mmazzarolo avatar Mar 20 '21 09:03 mmazzarolo

@mmazzarolo The issue that this PR fixes (#20951) is when window.history.scrollRestoration is set to 'manual' by the user, without enabling that experimental feature.

shuding avatar Mar 25 '21 13:03 shuding

@shuding understood. Sorry, I missed it 👍

mmazzarolo avatar Mar 25 '21 14:03 mmazzarolo

@shuding is this still planned to be reviewed and merged?

karlhorky avatar Nov 03 '22 10:11 karlhorky

@JanKaifer oh you closed it! 👀

Is the functionality added in this PR already now in canary (eg. through some other PR)? Or is there something which makes this irrelevant / unnecessary?

Last we checked (before Next.js 13 + app directory), the built-in scroll restoration in Next.js (both standard scroll restoration and the experimental.scrollRestoration flag) was still not working properly when navigating back using the browser Back button.

We have a workaround mostly based on this Gist, but it seems unusual to have to do this.

karlhorky avatar Feb 02 '23 14:02 karlhorky

Hi, this PR was stale and it's not a priority now. If you would like to pursue this PR, feel free to fork it and create your own.

jankaifer avatar Feb 02 '23 14:02 jankaifer