Next 12.3.1 breaks smooth-scrolling, change made in 12.3.1-canary.4, woks in 12.3.1-canary.3 and before

[gurkerl83]

Verify canary release

• I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
Platform: darwin
Arch: x64
Version: Darwin Kernel Version 21.6.0: Sat Jun 18 17:07:25 PDT 2022; root:xnu-8020.140.41~1/RELEASE_X86_64
Binaries:
Node: 16.13.0
npm: 8.1.0
Yarn: 3.2.3
pnpm: N/A
Relevant packages:
next: 12.3.1-canary.3
eslint-config-next: N/A
react: 0.0.0-experimental-8951c5fc9-20220915
react-dom: 0.0.0-experimental-8951c5fc9-20220915

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

@timneutkens @cramforce The following commit completely disables smooth-scrolling- #40642. We use the following custom hook here to trigger the scroll-behaviour we want. We do not have anything else like a global css which sets the scroll-behaviour on html.

The scroll-behaviour becomes non-deterministic in version 12.3.1-canary.4, the previous version 12.3.1-canary.3 works perfectly. The latest release 12.3.1 is also affected.

import { MittEmitter } from '@app/render-utils';
import { useEffect } from 'react';

/**
 * The scrolling behavior is always set to smooth, when a page mounts.
 * On route change, the scrolling behavior switches to auto before it snaps back to smooth.
 *
 * Note:
 * Do not use the nextJs hook "use-router" in this custom hook; constant re-renders get executed;
 * Unfortunately, it seems there are some bugs in the router implementation (asPath / hash-change).
 *
 * Instead, the nextJs router events object from the _app component is given as an argument.
 */

export const useSmoothScroll = (events: MittEmitter) => {
  const setSmoothScroll = (isSmooth: boolean) => {
    document.documentElement.style.scrollBehavior = isSmooth
      ? 'smooth'
      : 'auto';
  };

  useEffect(() => {
    setSmoothScroll(true);

    const handleRouteChangeStart = () => setSmoothScroll(false);
    const handleRouteChangeComplete = () => setSmoothScroll(true);

    events.on('routeChangeStart', handleRouteChangeStart);
    events.on('routeChangeComplete', handleRouteChangeComplete);

    return () => {
      events.off('routeChangeStart', handleRouteChangeStart);
      events.off('routeChangeComplete', handleRouteChangeComplete);
    };
  }, []);
};

Can you please provide feedback on how to enable scroll-behaviour using the hook again?
Thx!

Expected Behavior

Identical behaviour like in 12.3.1-canary.3

Link to reproduction

I will not create a reproducer for things y break; and smooth-scrolling is something which broke a few times in the past.

To Reproduce

Use the hook from above in _app.

[gurkerl83]

@timneutkens @cramforce It seems that several people confirm this problem. Have you taken a look at this problem yet?

[noxzym]

any update for this?

[timneutkens]

@gurkerl83 I'm trying to understand what the problem is with the change that was landed, it implements exactly the behavior that is intended based on your hook: before it scrolls to top during navigation it sets it to auto, after it scrolled it sets it to what the value was before scrolling. Seems you can remove that hook and it'll work the way you want it to work?

[gurkerl83]

@timneutkens I disabled the hook, and configured scrolling with css. The result was really not the same, compared with the hook enabled using a nightly version of Next right before the change landed. For the next two weeks I’m traveling without any device on hand, but I can provide a reproducer afterwards. Thx for looking into that!

[gurkerl83]

@timneutkens I spent a bit of time on this problem yesterday.

before it scrolls to top during navigation it sets it to auto, after it scrolled it sets it to what the value was before scrolling.

In fact, the implementation seems to be wrong, as the scroll behaviour "auto" immediately or during scrolling switches back to "smooth" or the globally defined behaviour of the html tag.

Local experiments showed that the scrolling behaviour "auto" must be initiated at the start of a route change "routeChangeStart" and must be maintained until a route change is completed "routeChangeComplete".

Pre-condition:
The scroll behaviour "smooth" must either be defined globally in css for the html element. From the hook provided this is done once in the effect-lifecycle, however a user defined configuration seems better, especially as the behaviour can be integrated into Next as described in the next step.

Summary of the lifecycle:

1. Subscribe to routeChangeStart, routeChangeComplete
2. When the route change starts - switch to auto -> scroll immediately to the top / hold this value until the route change finishes.
3. When the route change finishes - switch back to smooth (or whatever was before or globally defined) -> e.g. hash changes are not route changes -> results in smooth scrolling.

In the next step I describe the necessary changes

• next/client/index.tsx -> component Root

In doRender, where the mounting of Root occurs

renderReactElement(appElement!, (callback) => (
    <Root router={router} callbacks={[callback, onRootCommit]}> // pass router

In the component Root
First variant, improved by second version further down

const setSmoothScroll = (isSmooth: boolean) => {
   document.documentElement.style.scrollBehavior = isSmooth ? 'smooth' : 'auto';
};

// We should ask to measure the Web Vitals after rendering completes so we
// don't cause any hydration delay:
useEffect(() => {
   measureWebVitals(onPerfEntry)

   const handleRouteChangeStart = () => {
      setSmoothScroll(false);
   }
   
   const handleRouteChangeComplete = () => {
      setSmoothScroll(true);
   }

   router.events.on('routeChangeStart', handleRouteChangeStart);
   router.events.on('routeChangeComplete', handleRouteChangeComplete);

   return () => {
      router.events.off('routeChangeStart', handleRouteChangeStart);
      router.events.off('routeChangeComplete', handleRouteChangeComplete);
   };
}, [])

Second variant, remember the initial scroll-behaviour defined for a global css html tag

const refScroll = React.useRef(null)

// We should ask to measure the Web Vitals after rendering completes so we
// don't cause any hydration delay:
useEffect(() => {
   measureWebVitals(onPerfEntry)

   const handleRouteChangeStart = () => {
      refScroll.current = document.documentElement.style.scrollBehavior;
      document.documentElement.style.scrollBehavior = 'auto';
   }
   
   const handleRouteChangeComplete = () => {
      document.documentElement.style.scrollBehavior = refScroll.current;
   }

   router.events.on('routeChangeStart', handleRouteChangeStart);
   router.events.on('routeChangeComplete', handleRouteChangeComplete);

   return () => {
      router.events.off('routeChangeStart', handleRouteChangeStart);
      router.events.off('routeChangeComplete', handleRouteChangeComplete);
   };
}, [])

Revert to previous code at the end of onHeadCommit

if (input.scroll) {
   window.scrollTo(input.scroll.x, input.scroll.y);
}

Probably the identical logic is also necessary in the file (not tested as this is probably due to Next's new layout system)

• next/client/components/layout-router -> component InnerLayoutRouter

Furthermore, the function handleSmoothScroll, which is called in scrollToHash (file next/shared/lib/router.ts) is not needed.

What do you think, I can provide a pr with those changes?

Thx!

[wyattjoh]

Hi @gurkerl83! I've attempted to reproduce the issue you described with the scrolling behavior, but I'm having issues with finding any issue.

You can see this reproduction over here https://nextjs-scroll-repro.vercel.app/ and can see that between the navigations, there is no issues.

[gurkerl83]

@wyattjoh The issue is when you have two pages (routes), both huge in space.

Obviously, smooth scrolling on one page is not a problem and it works.

But when you are scrolled very far from the top of page A and then execute a route change to the page B (also very long) during navigation, the page scrolls all the way up from the previous scroll position of Page A to the top (newly loaded page B); this has to be immediate, with "auto" activated until the page loading is finished.

The vertical amount of space of a page and where the scroll position is when an navigation attempt is made is really the key here to reproduce the issue.

I hope this makes sense.

Thx!

[wyattjoh]

Is it possible to fork my reproduction to create an appropriate reproduction of the issue?

[gurkerl83]

I think i see this already in your reproducer. Make the sections larger, scroll down in one page and execute a route change to the other page (not a hash change). You will see a scroll during navigation to the top and this is what needs to be immediate.

[gurkerl83]

@wyattjoh Here is a video with your reproducer in action illustration the problem. I adjusted the height of the sections to make it clear. Imagine when you have larger pages such as blog posts or a e-commerce pages showing dozen of items, changing a route causes a smooth transition to the top, which is really annoying, and this has to be immediate.

Screen.Recording.2022-12-01.at.23.57.02.mov

Thx!