[BUG] Setting initial scrolling position is racy

[vyivel]

Describe the bug

Occasionally initialScrollTop and initialTopMostItemIndex have no effect, presumably due to the scrolling being applied before the scrollHeight of the element is updated. This is easier to reproduce with multiple lists.

Related: #874

Reproduction

https://codesandbox.io/p/sandbox/laughing-jang-cycy4n

To Reproduce

Just load the page. Depending on the PC specs and the browser used, this might take multiple reloads or not work at all.

Expected behavior

Screenshots

Firefox 111.0.1: some of the first lists aren't scrolled.

When initialScrollTop is used, none of the lists are scrolled.

Chromium 111.0.5563.146: seems to work fine with initialTopMostItemIndex.

When initialScrollTop is used, only the first list isn't scrolled; this behavior is consistent across reloads.

Desktop

• OS: Fedora 37
• Browser: multiple (see above)

Additional context

I can't reproduce this in qutebrowser.

---

With the following patch:

diff --git a/src/hooks/useChangedChildSizes.ts b/src/hooks/useChangedChildSizes.ts
index a89de52a..9a53faa6 100644
--- a/src/hooks/useChangedChildSizes.ts
+++ b/src/hooks/useChangedChildSizes.ts
@@ -42,6 +42,10 @@ export default function useChangedListContentsSizes(
         ? window.innerHeight
         : scrollableElement.offsetHeight
 
+      console.log(`useChangedListContentsSize:
+        scrollTop = ${scrollTop},
+        scrollHeight = ${scrollHeight},
+        ranges = ${ranges != null ? (ranges.length == 0 ? "<empty>" :ranges.map((sr) => `${sr.startIndex}, ${sr.endIndex}, ${sr.size}`).join("; ")) : "null"}`);
       scrollContainerStateCallback({
         scrollTop: Math.max(scrollTop, 0),
         scrollHeight,
diff --git a/src/hooks/useScrollTop.ts b/src/hooks/useScrollTop.ts
index a415cfce..dbff3204 100644
--- a/src/hooks/useScrollTop.ts
+++ b/src/hooks/useScrollTop.ts
@@ -96,6 +96,13 @@ export default function useScrollTop(
     // avoid system hanging because the DOM never called back
     // with the scrollTop
     // scroller is already at this location
+    console.log(`scrollToCallback:
+      offsetHeight = ${offsetHeight},
+      scrollHeight = ${scrollHeight},
+      approximatelyEqual(offsetHeight, scrollHeight) = ${approximatelyEqual(offsetHeight, scrollHeight)},
+      location.top = ${location.top},
+      scrollTop = ${scrollTop},
+      (location.top === scrollTop) = ${location.top === scrollTop}`)
     if (approximatelyEqual(offsetHeight, scrollHeight) || location.top === scrollTop) {
       scrollContainerStateCallback({ scrollTop, scrollHeight, viewportHeight: offsetHeight })
       if (isSmooth) {

The result is the following; the logs suggest that scrollToCallback() is called too early, before the proper scrollHeight is set.

---

Please tell if you need more information.

[petyosi]

Confirming, I managed to reproduce on my side. Will look into it and update the issue, thanks for looking into the source code for the additional hints.

[github-actions]

🎉 This issue has been resolved in version 4.2.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[vyivel]

initialScrollTop seems to be fixed with 42c8a03, but initialTopMostItemIndex is still racy in Firefox.

[petyosi]

@vyivel so these are actually two issues. initialScrollTop and initialTopMostItemIndex follow a separate implementation logic, and I strongly recommend against using initialScrollTop (discussed in some issues here).

• the initialScrollTop was consistently broken across browsers. I was blind-sided as the test case scenario had another flag set that made it work there. The fix above identified the problem and should address it.

• the initialTopMostItemIndex in firefox seems to be some sort of a heisenbug. I see it rarely in the sandbox, but when I took your sandbox and added it to the playground, I can't seem to reproduce the problem locally.

If there's anything that can make the problem consistent and reproducible in the local environment, let me know.

[github-actions]

🎉 This issue has been resolved in version 4.3.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[petyosi]

@vyivel I reproduced (and, hopefully, fixed) that while working on another thing. Please let me know if you still experience it.

[vyivel]

Tried different cases with 4.3.0, everything works as expected, so the problem seems to be fixed. Thanks for your work!