callstack / react-native-testing-library

🦉 Simple and complete React Native testing utilities that encourage good testing practices.
https://callstack.github.io/react-native-testing-library/
MIT License
3.02k stars 264 forks source link

`fireEvent.scroll` doesn't update the FlatList's render window on React Native 0.73 #1540

Closed j-piasecki closed 7 months ago

j-piasecki commented 7 months ago

Describe the bug

Since https://github.com/facebook/react-native/pull/38529 React Native's VirtualizedList doesn't update its contentLength when handling the scroll event. This means that using fireEvent.scroll(...) is not enough to render items outside the initialNumToRender - invoking onContentSizeChange is also necessary.

Expected behavior

Steps to Reproduce

Try the following snippet on RN 0.72 and 0.73. Without the commented-out line, the test will be failing on 73 but not on 72.

import React from 'react';
import {View, FlatList, Text} from 'react-native';
import {render, screen, fireEvent} from '@testing-library/react-native';

const DATA = new Array(100).fill(0).map((_, i) => `Item ${i}`);

function Item({title}: {title: string}) {
  return (
    <View>
      <Text>{title}</Text>
    </View>
  );
}

function Scrollable() {
  return (
    <View style={{flex: 1}}>
      <FlatList
        testID="test-flatlist"
        data={DATA}
        renderItem={x => <Item title={x.item} />}
        initialNumToRender={10}
      />
    </View>
  );
}

test('example', async () => {
  render(<Scrollable />);

  const flatlist = screen.getByTestId('test-flatlist');

  const item1 = screen.getByText('Item 0');
  const item2 = screen.getByText('Item 7');

  fireEvent.scroll(flatlist, {
    nativeEvent: {
      contentOffset: {
        y: 480,
      },
      contentSize: {
        height: 480,
        width: 240,
      },
      layoutMeasurement: {
        height: 480,
        width: 240,
      },
    },
  });

  // fireEvent(flatlist, 'onContentSizeChange', 240, 480);

  await new Promise(x => setTimeout(x, 100));

  const item3 = screen.getByText('Item 15');
});

Screenshots

Versions

npmPackages:
    @testing-library/react-native: ^12.4.1 => 12.4.1
    react: 18.2.0 => 18.2.0
    react-native: 0.73.0-rc.6 => 0.73.0-rc.6
    react-test-renderer: 18.2.0 => 18.2.0
mdjastrzebski commented 7 months ago

@j-piasecki Hmmm, I'm quite surprised to learn that fireEvent.scroll did update anything in any RN version 🧐

Fire Event API is very simple and works by finding a matching event handler (here: onScroll) and invoking it with the supplied parameters. The fact that it did work in RN 0.72 would suggest that RN itself (or some mock provided by RN) would provide an onScroll event handler on host ScrollView rendered by composite FlatList, which in turn triggered some FlatList logic.

Fire Event API is not intended for RN env simulation, for that purpose, we have created the User Event API. Take a look at the scrollTo function, which should provide much better simulation by emitting multiple scroll-related events trying to mimic RN runtime behavior. Be aware that the current implementation of it is quite basic and has been based only on plain ScrollView interactions. However, it should emit both onScroll as well as onContentSizeChange. If you have knowledge and interest in supporting this feature in FlatList, I would encourage you to submit a PR for that. From my side, I offer help in reviewing the code and explaining RNTL intricacies if necessary.

j-piasecki commented 7 months ago

Fire Event API is very simple and works by finding a matching event handler (here: onScroll) and invoking it with the supplied parameters. The fact that it did work in RN 0.72 would suggest that RN itself (or some mock provided by RN) would provide an onScroll event handler on host ScrollView rendered by composite FlatList, which in turn triggered some FlatList logic.

That's exactly what's been happening. VirtualizedList was updating its content length inside onScroll handler, but is no longer doing that since 0.73.

Fire Event API is not intended for RN env simulation, for that purpose, we have created the User Event API. Take a look at the scrollTo function, which should provide much better simulation by emitting multiple scroll-related events trying to mimic RN runtime behavior.

Thanks! I'll look into that more.

If you have knowledge and interest in supporting this feature in FlatList, I would encourage you to submit a PR for that. From my side, I offer help in reviewing the code and explaining RNTL intricacies if necessary.

It's possible that scrollTo already handles that - the docs mention that it should work with FlashList. I'll get back to this issue after some testing.

j-piasecki commented 7 months ago

I've looked into this more, and it seems like scrollTo doesn't handle that case 😞. I think the reason is twofold:

  1. When scrollTo builds scroll events, it passes zeros to all properties except contentOffset - main offenders here are contentSize and layoutMeasurement: https://github.com/callstack/react-native-testing-library/blob/3ba97e35b904f385644d35a5d78cc3ed583f81ac/src/user-event/event-builder/scroll-view.ts#L21-L25
  2. Because of the change mentioned in the issue description, VirtualizedList no longer handles contentSize inside its onScroll handler, meaning that its onContentSizeChange handler also would need to be invoked. Normally, this is done by ScrollView inside its onLayout callback, however the ScrollView is mocked and doesn't handle onLayout at all.

At this point, I'm not sure whether it should be handled by the library or by the users on a case-by-case basis, since it's necessary only for VirtualizedLists and the documentation clearly states that it focuses on the host ScrollView component.

mdjastrzebski commented 7 months ago

@j-piasecki we could extend scrollTo API to allow passing contentSize & layoutMeasurement. If not passed they would default to zero.

j-piasecki commented 7 months ago

we could extend scrollTo API to allow passing contentSize & layoutMeasurement. If not passed they would default to zero.

That would be great! This change alone should make it work with VirtualizedLists for RN 0.72 and below, however for 0.73 triggering onContentSizeChange is also required. Do you think it should be included in the scrollTo API or should that be a user's responsibility?

mdjastrzebski commented 7 months ago

Here's how we could implement this:

  1. user would optionally pass contentSize and layoutMeasurement values to scrollTo call:

    await userEvent.scrollTo(flatlist, {
      y: 480,
      contentSize: { height: 480, width: 240 },
      layoutMeasurement: { height: 480, width: 240 },
    });
  2. User Event would pass relevant values to proper individual events.

Both values seem to be stable for the duration of user interaction, so we just forward them. If User Event lacks certain event in the sequence, the we could as them.

@j-piasecki Would you be able to submit a PR for that?

j-piasecki commented 7 months ago

Sorry for the delay, the PR is up