fix(gatsby): scroll restoration issue in browser API

[vrabe]

Description

getSavedScrollPosition() in shouldUpdateScroll doesn't pass key to SessionStorage, so the storage can't find the stored position.
I pass args.key as key so that SessionStorage can generate the correct key.

Related Issues

relates to #27349

[vladar]

Thanks for the PR!

Have you tried it with the original reproduction? When I run it, I get this runtime error (in gatsby develop):

Uncaught TypeError: Invalid attempt to spread non-iterable instance.
In order to be iterable, non-array objects must have a [Symbol.iterator]() method.
    at _nonIterableSpread (nonIterableSpread.js:2)
    at _toConsumableArray (toConsumableArray.js:6)
    at gatsby-browser.js:20

[vrabe]

It seems that the storage was stored coordination ([0, Y_value]) but now it only stores y values.

If you modify line 20 in gatsby-browser.js to:

window.setTimeout(
  () => window.scrollTo(...([0, savedPosition] || [0, 0])),
  transitionDelay
)

It'll work.

[vladar]

Yeah, but this way it is a breaking change. We should try to preserve the old behavior (which is also documented here: https://www.gatsbyjs.com/docs/browser-apis/#shouldUpdateScroll)

[vrabe]

You are right. I change a little to keep the old behavior.

[vladar]

Sorry, missed your last comment. I Will review it later this week. Thanks!

[vladar]

I tried it with the reproduction and it works 🎉 Thanks for the PR and sorry for the delay with the review!

Also adding some context on why it has to be done this way. We save the state here and only save the Y coordinate:

gatsby/packages/gatsby-react-router-scroll/src/scroll-handler.tsx

Line 32 in 43b2f10

this._stateStorage.save(this.props.location, key, window.scrollY)

Also, the initial key is taken from the location.key itself.

[pieh]

Published in [email protected]

[trevorblades]

It seems that the storage was stored coordination ([0, Y_value]) but now it only stores y values.

If you modify line 20 in gatsby-browser.js to:

window.setTimeout(
  () => window.scrollTo(...([0, savedPosition] || [0, 0])),
  transitionDelay
)

It'll work.

Edit: I misread the above comments, and looks like my comment below doesn't make sense anymore if the original behaviour was preserved.

The statement [0, savedPosition] || [0, 0] will always return the left side of the || operator, since an array is never falsey. I think what you meant to write was [0, savedPosition || 0]. However, since savedPosition is no longer an array, we can just pass the Y value directly to the second argument of window.scrollTo now:

() => window.scrollTo(0, savedPosition || 0)