Scroll position is restored when not needed

[catamphetamine]

I found a bug in scroll position restoration in found library.

The scroll position is stored in Session storage:

@@scroll|/vg = [0,666.5] 

where /vg is the page path.

If a user opens a page, then goes to the URL bar and enters another page URL, then goes to the URL bar again and enters the initial page URL — the page loads and the scroll is at where the user left it initially, not at the top of the page, because Session storage still has that entry. And the entry has no random-generated hash or anything.

[taion]

That's a good catch. The issue here is that the initial location doesn't get supplemented with a randomly-generated key, as we didn't want to trigger an immediate navigation action on page load.

Not quite sure how to deal with this one.

That said, this isn't specific to Found. The same thing will happen on e.g. most sites that use anything like this logic for determining page key, including Gatsby sites. You can see this behavior on e.g. https://reactjs.org.

cc @KyleAMathews @jquense – older versions of React Router actually would immediately redirect to a version of the page with a random key set on load, but I think users tended to not like that behavior much. Not sure what the best way to deal with it is. I don't think there's a way to deal with it inside this library, though, but it is a weird bug.

[catamphetamine]

@taion Yeah, I recall that in react-router@3 every location had its own random-generated key and I didn't like the idea of having it.

For the current situation, I was thinking about clearing all @@scroll| keys from Session storage on page load:

window.onload = function onPageLoad() {
  forEach((key) => {
    if (key.indexOf('@@scroll|') === 0) {
      sessionStorage.removeItem(key)
    }
  }
}

function forEach(func) {
  let i = 0
  while (i < sessionStorage.length) {
    const key = sessionStorage.key(i)
    func(key)
    i++
  }
}

[taion]

That's not quite the same, though. To emulate native browser scroll behavior, you do want to preserve scroll position if someone reloads the page.

[catamphetamine]

@taion Aaah, I see.

I wonder what's the point of preserving scroll position in browsers when a user reloads a page though. What's the rationale, the motivation. Whatever.

[taion]

And to be clear, we do still attach keys on navigation to locations after the first. But we don't do this for the first location because it would require triggering a history action to get it stamped on.

[hedgepigdaniel]

Whats wrong with storing a random key on each location entry?

That makes perfect sense to me - a key to identify each one so that even if the URL is the same, there is something to distinguish 2 separate location entries that should not share the same scroll position.

Hard to see how this library could work properly if the locations provided to it are not unique.

[taion]

Farce adds unique keys to all locations that it generates.

However, for the initial page load, I'm not aware of a way to update the location state. Old versions of history used to deal with this by immediately calling replaceState, but that led to weird bugs.

[hedgepigdaniel]

What kind of bugs did it cause?

[taion]

I honestly can't remember any more. It was enough that they took it out in history v4, though.