Fix artificial scroll limit expanding on scroll to un-measured area (2nd of 4 problems that cause #1254)

[LucioChavezFuentes]

I found 4 problems that cause the Inverted VirtualizedList to go wild (#1254). This PR will explain and fix the 2nd of 4 problems found.

If we fix the following 4 problems, issue #1254 will be fixed and FlatList’s scroll will be more stable:

1. Inverted VirtualizedList has incorrect baseline measurement for its items’ offset —> Problem Explanation and Solution in this PR.

---

Update:

PR 2, 3, and 4 will fix 'Flatlist with expensive items breaks scroll' issue. PR 1 is enough to fix the Inverted Flatlist issue if your Flatlist's items are cheap to mount.

---

2. $lead_spacer expands scroll artificially when Inverted VirtualizedList is mounting new items —> Problem Explanation and Solution below.

3. VirtualizedList skip items for offset measuring when the user scrolls very fast while new items are mounted and measured (this happens also for normal lists) —> Problem Explanation and Solution in this PR.

4. VirtualizedList gets offsets equal to zero for items that are not the list's first item (this happens also for normal lists) —> Problem Explanation and Solution in this PR.

Video of Inverted FlatList with all problems fixed:

2022-10-23.17-49-22.mp4

2nd Problem

$lead_spacer expands scroll limit artificially when Inverted VirtualizedList is mounting new items. (2nd of 4 problems that cause #1254)

In a FlatList with items with different heights, the scroll is limited to the last measured item. That’s because VirtualizedList (the inner FlatList's component) must measure the offset of mounted items before it tries to mount new items. In this way, all items in the virtual area are measured and the _frames object is complete.

The scroll limit should be only expanded when the user tries to scroll beyond the scroll limit (unmeasured area) and then new items are mounted and their offsets are measured and saved in _frames. When recently mounted items are measured, the scroll limit is expanded to the new latest measured item, then the user will be ready to keep scrolling to unmeasured area and repeat the process to keep expanding the scroll limit until the last item is met.

Sometimes users may “overscroll” to unmeasured areas while previously mounted items haven't been measured yet, and VirtualizedList starts skipping items for measuring and measures others that shouldn’t be measured yet, leading to a fragmented map of offsets (e.g. we have offset values for …31, 32, 33, then jumps to 37, 38…).

Why do users “overscroll”?

Flaltlist constantly mounts and unmounts items. There is a white space mounted on our list called $tail_spacer. It helps us to fill with white space the top outside of our virtual area (because it’s an inverted list, for normal lists it fills the bottom), where items were previously mounted and measured but unmounted afterward, so the scroll limit sticks to our latest measured item offset.

$tail_spacer height depends on how much area we had measured, but it reduces to zero when the latest measured item is mounted and the user pretends to scroll to an unmeasured area looking for more items. When $tail_spacer's height is zero, the scroll is limited and prevents the user from “overscroll”. This gives time to VirtualizedList to properly mount and measure new items to then use their offsets as our new scroll limit.

Sometimes $tail_spacer has a height greater than zero when it should be zero, allowing the user to “overscroll”.

Why sometimes $tail_spacer has a height greater than zero when it should be zero?

On some updates, $tail_spacer gets a negative value for its height, which is an invalid value. Invalid values will be ignored and $tail-spacer’s height will set the valid value of the previous state, instead of setting it to zero.

Solution:

The solution is simple. Check tailSpacerLenght, if it is below zero then set it to zero:

react-native-web/src/vendor/react-native/VirtualizedList/index.js

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2d43bfb:

Sandbox	Source
react-native-web-examples	Configuration

[roryabraham]

I think this change belongs in the upstream React Native, not react-native-web.

The upstream implementation of VirtualizedList has recently changed, so unless I'm mistaken the new place to make this change would be right here, and the following change would do:

- const spacerSize =
-             lastMetrics.offset + lastMetrics.length - firstMetrics.offset;
+ const spacerSize = Math.max(
+     lastMetrics.offset + lastMetrics.length - firstMetrics.offset,
+     0,
+ );

[LucioChavezFuentes]

I think this change belongs in the upstream React Native, not react-native-web.

The upstream implementation of VirtualizedList has recently changed, so unless I'm mistaken the new place to make this change would be right here, and the following change would do:

- const spacerSize =
-             lastMetrics.offset + lastMetrics.length - firstMetrics.offset;
+ const spacerSize = Math.max(
+     lastMetrics.offset + lastMetrics.length - firstMetrics.offset,
+     0,
+ );

@roryabraham I managed to run the VirtualizedList's latest changes on Web and this solution is no longer required but I don't know exactly why. I am still learning about the new VirtualizedList. The rest of the solutions still apply and fix the scroll issues in the new VirtualizedList on Web. I am on my way to propose these changes to react-native.

[necolas]

@LucioChavezFuentes thanks for working through this. And I forgot to mention this earlier but your bug reports with diagrams, details, and videos are really excellent. Thank you

[LucioChavezFuentes]

I think this change belongs in the upstream React Native, not react-native-web.
The upstream implementation of VirtualizedList has recently changed, so unless I'm mistaken the new place to make this change would be right here, and the following change would do:

- const spacerSize =
-             lastMetrics.offset + lastMetrics.length - firstMetrics.offset;
+ const spacerSize = Math.max(
+     lastMetrics.offset + lastMetrics.length - firstMetrics.offset,
+     0,
+ );

@roryabraham I managed to run the VirtualizedList's latest changes on Web and this solution is no longer required but I don't know exactly why. I am still learning about the new VirtualizedList. The rest of the solutions still apply and fix the scroll issues in the new VirtualizedList on Web. I am on my way to propose these changes to react-native.

I think this change belongs in the upstream React Native, not react-native-web.
The upstream implementation of VirtualizedList has recently changed, so unless I'm mistaken the new place to make this change would be right here, and the following change would do:

- const spacerSize =
-             lastMetrics.offset + lastMetrics.length - firstMetrics.offset;
+ const spacerSize = Math.max(
+     lastMetrics.offset + lastMetrics.length - firstMetrics.offset,
+     0,
+ );

@roryabraham I managed to run the VirtualizedList's latest changes on Web and this solution is no longer required but I don't know exactly why. I am still learning about the new VirtualizedList. The rest of the solutions still apply and fix the scroll issues in the new VirtualizedList on Web. I am on my way to propose these changes to react-native.

@roryabraham I was wrong, this solution does provide more stability, especially on slow devices or when the user scrolls very fast. I will keep it for the react-native proposal.