Fix VirtualizedList skip measure items - Improve scroll experience

[LucioChavezFuentes]

After investigation and feedback I have a new proposal of pull requests to fix the Inverted VirtualizedList in web. Some of the root issues are in react-native-web's end, another issue is fixed updating VirtualizedList from react-native:

• Problem 1: VirtualizedList's code relies on that onLayout measures items without considering transformations —> Problem Explanation and Solution in this PR.

• Problem 2: Exists inaccurate measures in onLayout when the setTimeout is executed and any of the targeted nodes (node or relativeNode) for measure are unmounted. —> Problem Explanation and Solution in this PR (same PR that solves problem 1).

• Problem 3: The scroll position can be forced to areas where items have not been measured and the list skips measuring items if the user scrolls too fast. —> Problem Explanation and Solution below.

---

Problem 3

Even if we measure correctly the items of VirtualizedList we still have a problem with the scroll experience if the user scrolls too fast.

The react-native's VirtualizedList updated code fixes this problem but the react-native-web's wheel patch for inverted VirtualizedList opens the possibility for users to force scroll any part of the list and not give enough time for the list to measure items to map their position.

Solution

1. Update VirtualizedList from react-native (included in this PR)
2. Limit the scroll position based on VirtalizedList's totalCellLength (the sum of heights of all items measured)

With all solutions applied and updating VirtualizedList from react-native we have a solid VirtualizedList in web, inverted or non-inverted.

Thank you for reading @necolas, if you have any questions or concerns please let me know how I can help.

[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 f5b3b8a:

Sandbox	Source
react-native-web-examples	Configuration

[janicduplessis]

Looks good. I wonder if we would we be able to vendor https://github.com/facebook/react-native/tree/main/packages/virtualized-lists package directly instead?

[LucioChavezFuentes]

Looks good. I wonder if we would we be able to vendor https://github.com/facebook/react-native/tree/main/packages/virtualized-lists package directly instead?

I think we can, I will give a try. To be honest I still learning how the files are organized and exported in RN vs RNW.

[necolas]

I appreciate all the hard work you've put into identifying and fixes issues in the RN lists.

But I would like to stop forking FlatList/VirtualizedList, which is why I've been trying to get those modules packaged out of RN and imported into RNW. Then all patches can be made in once place (the RN repo) which should simplify making fixes and help cross-platform apps with consistency. So I'm unlikely to merge this PR at this time. I understand it's probably also a challenge to get fixes to the lists merged in the RN repo at the moment (sorry about that), but having only one place to get changes reviewed and merged should at least make things better going forward.

Let me know if there's anything else I can do

[LucioChavezFuentes]

Thank you for review @necolas but I still need to understand the workflow between RN and RNW.

1. If I want to make changes to VirtualizedList I should apply a PR to to RN's VirtualizedList. And RNW pulls the changes from RN?
2. As for the RNW's patches (like the inverted wheel) they are exclusive of RNW repo? If that is true then any PR that make changes to those patches goes to RNW?

This PR is only pulling changes from RN and making a change to a RNW patch.

[necolas]

There's an issue in RN to move Animated, VirtualizedList, and FlatList to packages that get published to npm. facebook/react-native#35263

Once this is complete, RNW will delete the forked modules and import the same npm packages used by RN. All patches for all platforms will need to be made in the packages in the RN monorepo. https://github.com/facebook/react-native/tree/main/packages/virtualized-lists

RNW-specific patches for the inverted wheel will be lost. Basically, a fresh start.

[necolas]

I see that this PR is the result of a bunch of back-and-forth work, and that it's part of a larger Expensify issue that you've been working on for months. Since the progress of publishing those packages in RN is going pretty slowly, I'll merge this PR and make it the last update from the RNW side. Any virtualizedlist features that exist only in RNW should be ported to the RN version in the coming weeks, if you would like to preserve them once the package switch occurs.

[necolas]

This needs a rebase please

[necolas]

I didn't see where this gets used

[LucioChavezFuentes]

Here, inside VirtualizedList/index.js

[necolas]

I'm also open to including this patch for VirtualizedList. Let me know if it looks good and is something Expensify wants #2436

Then all the current patches related to scroll wheel will be included at least, and porting anything over to RN might be easier.

[LucioChavezFuentes]

@necolas this PR also has been rebased!

[LucioChavezFuentes]

Should I move the files to a virtualized-lists folder? Like this comment suggests?

[necolas]

I don't think that's necessary since no more changes will go into this fork. You should let me know about the other patches and issues I mentioned in both your PRs. Which ones are fixed by these PRs, and which ones should also be merged.

[necolas]

The flow tests are currently failing

[LucioChavezFuentes]

The flow tests are currently failing

Im on it, looks like there a lot of errors.

[LucioChavezFuentes]

@necolas I have a problem with the modules flow type:

Could you help me how to tackle this issue, please?

[LucioChavezFuentes]

This commit breaks any RNW app that uses react-native-gesture-handler and @react-navigation/native libraries. Had to revert the commit locally in order to test my PR in Expensify App. Should it be an issue in react-native-web or in the libraries?

[necolas]

People had 6 months to report that change to those libraries and refused to do so.

[LucioChavezFuentes]

@necolas I have a problem with the modules flow type:

Could you help me how to tackle this issue, please?

@necolas I added these files to fix the modules types problems. Is this ok?

[necolas]

Hmm, do you know why React Native doesn't have these errors despite not including these module declarations?

[LucioChavezFuentes]

Hmm, do you know why React Native doesn't have these errors despite not including these module declarations?

No, It's what I've been trying to figure out.

[LucioChavezFuentes]

@necolas I fixed the typing module from react native perspective. React-Native declares the types in .flowconfig. Also, It doesn't ignore the node_modules, except for specific modules.

This PR fixes #2432

[LucioChavezFuentes]

Hello, @necolas. This PR is ready, it fixes #2432
Let me know if can help with something else. I fixed the typing modules based on the react-native's flow configuration.

[LucioChavezFuentes]

Thank you @necolas, hope I was helpful.

[necolas]

Thanks! This patch is in release 0.19.2