Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
j-piasecki opened this issue Dec 4, 2023 · 7 comments · Fixed by #1543
Closed

Comments

@j-piasecki
Copy link
Contributor

Describe the bug

Since facebook/react-native#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
@j-piasecki j-piasecki changed the title 'fireEvent.scroll` doesn't update the FlatList's render window on React Native 0.73 fireEvent.scroll doesn't update the FlatList's render window on React Native 0.73 Dec 4, 2023
@mdjastrzebski
Copy link
Member

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

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:
    contentSize: { height: 0, width: 0 },
    layoutMeasurement: {
    height: 0,
    width: 0,
    },
  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
Copy link
Member

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

@j-piasecki
Copy link
Contributor Author

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
Copy link
Member

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 },
  });
  1. 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
Copy link
Contributor Author

Sorry for the delay, the PR is up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants