FlatList: onViewableItemsChanged is broken if pagingEnabled is true

[rnike]

The problem

Hi, we are using onViewableItemsChanged to do the impression tracking of items with FlatList, but we currently facing a problem with FlatList when pagingEnabled is true.

If pagingEnabled is true, we get all the items(include items not in the screen) from viewableItems in onViewableItemsChanged when scroll to the first item, get nothing when scroll to the second or after.

How to reproduce

Simplified test case:
Codesandbox

Steps to reproduce:

1. Go to this Codesandbox
2. Open the console log
3. Scroll the FlatList and see the log of viewableItems, viewableItems will log all the items when scroll to the first item, which is unexpected.
4. Remove pagingEnabled in FlatList
5. Scroll the FlatList again and see the viewableItems works as expected.

Expected behavior

Correctly get visible items from viewableItems in onViewableItemsChanged.

Environment (include versions). Did this work in previous versions?
Here is our currently used version

• React Native for Web (version): ^0.12.0
• React (version): 16.8.6
• Browser: Chrome

But this issue also occurs with the Codesandbox version

• React Native for Web (version): 0.13.16
• React (version): 16.13.1
• Browser: Chrome

Inspection
Took a look into the code using chrome debugger, found something wrong below

react-native-web/packages/react-native-web/src/exports/UIManager/index.js

Lines 27 to 31 in 6624a70

	const relativeRect = getBoundingClientRect(relativeNode);
	const { height, left, top, width } = getRect(node);
	const x = left - relativeRect.left;
	const y = top - relativeRect.top;
	callback(x, y, width, height, left, top);

In our case, the relativeRect.left has the same value of left returns by getRect(node), so all the items will have the same x which is 0, this will only happen if pagingEnabled is true

Workaround
Currently we override CellRendererComponent's onLayout to fix this problem

/**
 * This is used to fix the incorrect offset if pagingEnabled is true on web
 */
const FixCellOffsetOnWeb = (props) => {
  if (Platform.OS === "web") {
    const { onLayout, ...other } = props;

    const fixOffsetOnLayout = (e) => {
      if (onLayout) {
        onLayout({
          ...e,
          nativeEvent: {
            ...e.nativeEvent,
            layout: {
              ...e.nativeEvent.layout,
              // workaround: override the x offset
              x: other.index * ITEM_WIDTH 
            }
          }
        });
      }
    };

    return <View {...other} onLayout={fixOffsetOnLayout} />;
  }

  return <View {...props} />;
};

<FlatList
  CellRendererComponent={FixCellOffsetOnWeb}
  horizontal
  pagingEnabled
  // ... other props needed ...
/>

[necolas]

This is happening because:

1. An extra div is added around the items to support pagingEnabled.
2. React Native's onLayout is relative to the parent view.
3. But the parent view is different between web and native implementations.

This might also be an issue when using sticky headers: https://github.com/necolas/react-native-web/blob/0.14.6/packages/react-native-web/src/exports/ScrollView/index.js#L157-L177

I don't think there's a good way to deal with this other than: either remove the wrapping div and lose pagingEnabled etc support, or rewrite the VirtualizedList logic so it doesn't make this assumption.

React Native has not been good at patching JS code for other platforms, and VirtualizedList itself isn't great in terms of performance. But I'm not interested in forking and maintaining another slightly different VirtualizedList implementation when the JS one in React Native should be patched (or extracted to another package) to improve support for out-of-tree platforms.

[rnike]

@necolas Thanks for the explanation, currently we'll just stick with the workaround or using IntersectionObserver

[necolas]

Closing this old issue because VirtualizedList has been updated a few times since, and it will be developed exclusively out of the RN monorepo in the future. There may still be an issue in RNW related to how pagingEnabled is implemented, so feel free to create a new issue with latest info if needed.