fix: Several Fixes for Scroll Handling and Restoration

[blainekasten]

Description

Scroll handling and restoration within Gatsby is diverged from browser basics. There are a few basic things I've found while researching that Gatsby does not do the same (and this PR fixes)

1. url's with encoded hashes do not scroll to matching id's. (e.g., <h1 id="áccentuated">Scroll Here</h1>
2. url hashes are not scrolled to when navigating back (POP action)
3. url hashes are not navigated to on page load

While exploring this I also decided to clean up our code a bunch and remove some dependencies. Our usage of ScrollBehavior did not seem necessary. Maybe this is a foot gun and there were good reasons for it to exist.

I also added a useScrollRestoration hook that mirrors what Remix was selling for their scroll restoration inside of overflow containers.

I was also able to convert this all to TS.

Documentation

Maybe documentation to add here?

Related Issues

Fixes:

shouldUpdateScroll componentDidMount - #23792 (fixed) (breaking?)
accentuated doclinks - #23690 (fixed)
POP does not restore scrollPosition - #8019 (fixed)
doclinks not scrolled to on load - #21120 (fixed)

Additional Discussion

I think it would be worth considering, and exploring removing the shouldUpdateScroll API from gatsby-browser. In my research, it was only used in user land to patch our bugs.

[blainekasten]

A related question to this is that I don't know if we should let shouldUpdateScroll prevent scrolling to hashes. My inclination is to say no, but I don't know if that is a breaking change.

[wardpeet]

Nice work @blainekasten! This is a long-standing issue. For me POP does not restore scrollPosition is not working, it always resets to 0 for me.

Seems like key isn't updating on popstate. Probably because scroll is called in between.

[wardpeet]

We should make this separate functions instead of a class for modification, something for a future PR.

[blainekasten]

I think we maybe pass this to gatsby-browser functions. Something to consider

[wardpeet]

can you add a TODO to remove this in the next major? I don't see any usage of this file.

[blainekasten]

I think it's just for consumer usage. But once we support hooks, the hook approach should be the default. So yeah

[blainekasten]

I upped it one. I added a deprecation warning

[blainekasten]

@wardpeet wanna give it another test? I found a bug that I fixed. It seems to be working for me now!

[wardpeet]

Awesome, thanks! A few more

[blainekasten]

@wardpeet finally came back around to this and fixed everything up. Think it should be good to ship now! Take another review?

[wardpeet]

Thanks @blainekasten ! LGTM

[coreyward]

Heads up: this doesn't appear to have made it into the changelog, so it's hard to see which version of Gatsby this is fixed for.

[blainekasten]

@coreyward Fixed this in the above commit. Thanks for the call out. It was added in 2.23.5!

[blimpmason]

@wardpeet @blainekasten I'm not 100% sure but I suspect that API changes included in this merged PR have introduced issues with shouldUpdateScroll browser API. It also seems the browser API documentation on getSavedScrollPosition is outdated or incorrect (see #23842 (comment)).

It appears the scroll position is no longer saved in session storage as an [x, y] array, but rather as a single number for y position. All the Gatsby sites I've built with with animated page transitions no longer successfully restore scroll position on pop state — instead the user is always returned to the top of the page on browser back/forward. Up till now I've use this code snippet in gatsby-browser.js with no issue:

const transitionDelay = 500;

export const shouldUpdateScroll = ({
  routerProps: { location },
  getSavedScrollPosition
}) => {
  if (location.action === "PUSH") {
    window.setTimeout(() => window.scrollTo(0, 0), transitionDelay);
  } else {
    const savedPosition = getSavedScrollPosition(location);
    window.setTimeout(
      () => window.scrollTo(...(savedPosition || [0, 0])),
      transitionDelay
    );
  }
  return false;
};

After digging it appears the getSavedScrollPosition now requires both location and location.key as arguments, and returns a single number rather than x/y coordinates. So I've updated to this:

const transitionDelay = 500;

export const shouldUpdateScroll = ({
  routerProps: { location },
  getSavedScrollPosition
}) => {
  if (location.action === "PUSH") {
    window.setTimeout(() => window.scrollTo(0, 0), transitionDelay);
  } else {
    const savedYPosition = getSavedScrollPosition(location, location.key);
    window.setTimeout(
      () => window.scrollTo(0, savedYPosition || 0)),
      transitionDelay
    );
  }
  return false;
};

But unfortunately that update doesn't solve the issue either. It also seems that the scroll position is logged at a different time in the route change process because by the time it's called above (even with setTimeout delay matching the page transition speed), the savedYPosition comes back as 0 every single time.

This simple gatsby-browser trick used to work every time, as long as the setTimeout delay matches the speed of the page transition. Now it never seems to work. I couldn't find anything in the changelog that shed any light on the issue but found this PR as I browsed through the history of the files related to scroll handling and restoration.

[blimpmason]

A little more info — I was able to get the scroll restoration to work on popstate using the following workaround code that accesses the session storage directly instead of using getSavedScrollPosition, but it updates scroll position immediately when the page transition starts instead of waiting for the setTimeout. The UX is quite jerky and defeats the purpose of having a smooth page transition. That's what makes me think there's something different and breaking about when shouldUpdateScroll fires or whether it respects the setTimeout delay:

const getLatestSavedScrollPosition = ({ pathname, key }) => {
  const fullKey = `@@scroll|${pathname}|${key}`;
  return sessionStorage.getItem(fullKey);
};

export const shouldUpdateScroll = ({ routerProps: { location } }) => {
  console.log("shouldUpdateScroll");
  if (location.action === "PUSH") {
    window.setTimeout(() => window.scrollTo(0, 0), 600);
  } else {
    const yPosition = getLatestSavedScrollPosition(location);
    window.setTimeout(() => window.scrollTo(0, yPosition || 0), 600);
  }
  return false;
};

[wardpeet]

@blimpmason can you open an issue cause merged Prs are hardly checked