react-native icon indicating copy to clipboard operation
react-native copied to clipboard

[iOS/iPadOS] - `FlatList` refresh handling broken/dysfunctional

Open coolersham opened this issue 1 year ago • 23 comments

Description

Description

Since upgrading to the new architecture it seems like the refresh/pull-to-refresh feature of the FlatList is broken/not fully functional any longer. However, this is only the case if you place another FlatList OR ScrollView in the same view. Placing just a list in each view does not lead to problems.

Pulling down and refreshing works twice before being dysfunctional. Eventually, after a certain pattern, the functionality works again twice and then breaks again.

The sample code provided below works as intended on the old architecture.

Version

0.71.3

Output of npx react-native info

Not relevant for this issue.

Steps to reproduce

  1. Create a new react-native project with the help of the CLI, install all dependencies and pods and build the application on a iOS/iPadOS device.
  2. Paste the provided code into the App.tsx file
  3. Pull down the list and notice how the loading spinner is displayed for a brief time
  4. Use the Switch button to unmount the current view and mount another one containing two FlatList components and a ScrollView (for demonstration purposes).
  5. Pull to refresh on the most left FlatList (orange one) and once again use the Switch button
  6. Pull down the initial list and notice how the loading spinner is NOT displayed and the onRefresh handler is not called

Snack, code example, screenshot, or link to a repository

https://github.com/coolersham/NRNA-flatlist-reload-bug

import React, { useState } from "react"

import {
  FlatList,
  ScrollView,
  Text,
  TouchableOpacity,
  View,
} from "react-native"

export default function App(): JSX.Element {
  return (
    <View
      style={{
        flex: 1,
        padding: 48,
        alignItems: "center",
        backgroundColor: "white",
      }}
    >
      <ListDemo />
    </View>
  )
}

function ListDemo() {
  const data = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
  const [show, setShow] = useState(true)

  const style = { backgroundColor: "orange" }

  const switchButton = (
    <TouchableOpacity
      style={{
        height: 48,
        width: 300,
        alignItems: "center",
        backgroundColor: "red",
        justifyContent: "center",
      }}
      onPress={() => setShow((show) => !show)}
    >
      <Text style={{ color: "white" }}>Switch</Text>
    </TouchableOpacity>
  )

  if (show)
    return (
      <>
        {switchButton}
        <FlatList
          data={data}
          style={style}
          refreshing={false}
          renderItem={GenericListItem}
          onRefresh={() => console.log("List #1 refresh is called")}
        />
      </>
    )

  return (
    <>
      {switchButton}
      <View
        style={{
          flex: 1,
          flexDirection: "row",
        }}
      >
        <FlatList
          data={data}
          style={style}
          refreshing={false}
          renderItem={GenericListItem}
          onRefresh={() => console.log("List #2 refresh is called")}
        />

        {/* Breaks refresh handling of both lists occasionally */}
        <FlatList
          data={data}
          renderItem={GenericListItem}
          style={{ backgroundColor: "purple" }}
        />

        {/* The same applies to the normal ScrollView component */}
        <ScrollView style={{ backgroundColor: "blue" }}>
          {data.map((item) => (
            <GenericListItem key={item} />
          ))}
        </ScrollView>
      </View>
    </>
  )
}

function GenericListItem() {
  return (
    <View style={{ width: 300, height: 48 }}>
      <Text>Generic test item</Text>
    </View>
  )
}

coolersham avatar Feb 15 '23 18:02 coolersham

I was unable to reproduce despite following the steps. Tested on iPhone 13, iOS 15.2. onRefresh gets called every single time.

redpanda-bit avatar Feb 16 '23 21:02 redpanda-bit

@carlosalmonte04 Did you enable the new architecture, Fabric and Hermes and build the application afterwards?

coolersham avatar Feb 16 '23 21:02 coolersham

I did the steps below

  1. npx react-native init AwesomeApp --version=0.71.3
  2. cd in folder
  3. run npm install
  4. cd into ios
  5. run pod install
  6. copied and pasted provided code into App.tsx
  7. manual testing as described; - pulled the orange list, switched, pulled the new orange list, switched and pulled the original orange list

onRefresh got called every single time. Let me know if you'd like me to try a different way.

https://user-images.githubusercontent.com/25206487/219493096-aab9a704-96d7-4d3a-a27c-0f38567d8ca2.gif

redpanda-bit avatar Feb 16 '23 23:02 redpanda-bit

@carlosalmonte04 As stated in the main description of the issue, the problem is related to the new architecture. The template that the CLI retrieves in step 1. still uses the old architecture as a default setting.

You have to modify your fifth step to enable the new architecture and its corresponding parts. Instead of pod install do NO_FLIPPER=1 USE_FABRIC=1 USE_HERMES=1 RCT_NEW_ARCH_ENABLED=1 pod install.

Now you should be able to reproduce and perceive the bug. Please let me know if that has worked for you.

coolersham avatar Feb 17 '23 00:02 coolersham

Yes, that worked for me after using NO_FLIPPER=1 USE_FABRIC=1 USE_HERMES=1 RCT_NEW_ARCH_ENABLED=1 pod install. I am able to reproduce the issue.

redpanda-bit avatar Feb 17 '23 01:02 redpanda-bit

I'm adding a few breakpoints on the native side to see if I find something.

Also, adding a custom refreshControl component completely disables the onRefresh callback. Pulled down to refresh multiple times after adding the simple custom refreshControl component and onRefresh was not triggered.

redpanda-bit avatar Feb 17 '23 03:02 redpanda-bit

Might need to update designs temporarily to have a refresh button instead of pull-to-refresh. Or have a SectionList for the second list. Both approaches are disappointing but I can't remember the solution last time I experience this issue, I remember it had to do with styling issues.

<SectionList
        horizontal
        sections={[
          {
            data: [1, 2, 3],
            renderItem: item => (
              <FlatList
                contentContainerStyle={{
                  backgroundColor: bgColor[item.item],
                }}
                data={data}
                renderItem={() => (
                  <View style={{width: 100}}>
                    <GenericListItem />
                  </View>
                )}
              />
            ),
          },
        ]}
        refreshing={false}
        onRefresh={() => console.log('List #2 refresh is called')}
        renderItem={GenericListItem}
      />
    </>

👎

ezgif-2-a2900da66b

redpanda-bit avatar Feb 17 '23 19:02 redpanda-bit

Might need to update designs temporarily to have a refresh button instead of pull-to-refresh. Or have a SectionList for the second list. Both approaches are disappointing but I can't remember the solution last time I experience this issue, I remember it had to do with styling issues.

@carlosalmonte04 Thanks for looking into this and providing a possible workaround. However, as you mentioned, those approaches are disappointing and unsatisfactory.

I am not quite aware of how the responsibilities are divided in react-native matters, but could someone from the core or rather new architecture team look into this issue, please? Maybe I am doing something wrong or miss a piece of important information, but I think this functionality should not act like that. Even more, because it worked on the old architecture just fine.

Maybe @cortinico or would you mind delegating? Thanks!

coolersham avatar Feb 26 '23 18:02 coolersham

Again I kindly ask that someone from the core or new architecture team of react-native looks into this, please. Please just leave a comment, whether this issue will never be fixed or you do not have time for this/your priorities are spread differently, so that all people who are struggling with that issue know what the next steps will be.

coolersham avatar Mar 10 '23 13:03 coolersham

@coolersham did you end up finding a way to solve this?

redpanda-bit avatar Mar 25 '23 17:03 redpanda-bit

@carlosalmonte04 Sadly, I have not found a way to solve this particular broken feature. However, at this point, I am pretty confused and a little bit disappointed by the way the Facebook/React Native team is handling issues. Looking at the sheer number of open issues my hopes are low that we will receive an answer on this one soon.

I am well aware that there are probably more important things to focus on, but as mentioned before, even a short answer would be sufficient to let us know the next steps.

coolersham avatar Mar 29 '23 10:03 coolersham

However, at this point, I am pretty confused and a little bit disappointed by the way the Facebook/React Native team is handling issues

@coolersham we receive hundreds of issues every day and sadly our team is really small. We're trying to do our best to look at all of them and provide answers.

I've looked at your issue and sadly I don't have relevant context/suggestions to share at this stage.

cortinico avatar Mar 31 '23 11:03 cortinico

Thank you @cortinico, I totally understand your situation!

Alright. So will this ever be analysed in detail or does the community have to wait, whether this issue may be fixed over time or figure it out by themselves and create a PR for it?

coolersham avatar Apr 06 '23 09:04 coolersham

@coolersham having a reproducer project would be a first step to make investigation easier for us and also for folks in the community.

You can use this template: https://github.com/react-native-community/reproducer-react-native

If someone from the community is willing to investigate and fix this, we'll be more than happy to review & merge the PR

cortinico avatar Apr 06 '23 09:04 cortinico

@cortinico I already provided the code needed to experience this bug in my initial issue description and provided all other necessary information. As you can perceive in the timeline of this issue, the bug got already confirmed by @carlosalmonte04. If it would remove the little overhead needed to experience that bug, I could also create a reproduction repo and share it here.

coolersham avatar Apr 06 '23 09:04 coolersham

If it would remove the little overhead needed to experience that bug, I could also create a reproduction repo and share it here.

Please do 🙏 That would also help others which wish to investigate this

cortinico avatar Apr 06 '23 09:04 cortinico

Please do 🙏 That would also help others which wish to investigate this

@cortinico Here is the repository that reproduces this bug in the latest version of react-native (0.71.6).

coolersham avatar Apr 07 '23 12:04 coolersham

Any update on this?

Alecattie avatar Apr 17 '23 06:04 Alecattie

@Alecattie As no one commented here or linked a PR since I provided the reproduction repo - the answer is no.

coolersham avatar Apr 26 '23 10:04 coolersham

Almost seven months have passed since I opened this issue. Five months ago I provided a simple repro so this issue can be further investigated and fixed. Since then, nothing has happened from the maintainer or community side, except a quick fix in another issue, targeting the same problem #37308, which sadly brings along negative side effects and does not work in various scenarios either.

Has anyone found a viable solution that can be merged, or do we need to live without this expected standard from now on?

coolersham avatar Sep 13 '23 09:09 coolersham

Just upgraded the provided reproducer to the latest version of react-native (0.73.1) to check whether this issue has been fixed now. Turns out, this is sadly not the case. I have already moved on to an alternative implementation (functionality- and UI-wise). It still baffles me, that this bug is still around after more than 10 months and no one found a solution.

coolersham avatar Dec 30 '23 18:12 coolersham

Just upgraded the provided reproducer to the latest version of react-native (0.73.1) to check whether this issue has been fixed now. Turns out, this is sadly not the case. I have already moved on to an alternative implementation (functionality- and UI-wise). It still baffles me, that this bug is still around after more than 10 months and no one found a solution.

I'm also having this problem with the latest react-native version 😞 Any updates on this?

marcos-vinicius-mafei avatar Jan 19 '24 14:01 marcos-vinicius-mafei

This is still a problem on 0.73.6 on New Architecture. As is italic fonts are no longer italic and modals flashing when modal dismiss (but not if pressing Cancel button).

alex-strae avatar Apr 03 '24 10:04 alex-strae

Hi there, Sorry for the long silence, but as @cortinico mentioned above, we receive tons of issues, the team is small and we need to prioritize ruthlessly.

I'll start looking in to the issue today and for the next few days. Thanks @coolersham for the reproducer, I'll try to keep you all updated on the progress.


~~Update 1:~~ ~~I can repro the problem. I also noticed another difference between the Old and the New Architecture:~~ ~~* in the Old Architecture, the UIActivityIndicator is below the content view: when the content view is released, and it comes back to the top, it hides the UIActivityIndicator.~~ ~~* in the New Architecture, the UIActivityIndicator is in front of the content view: when the content view is released, and it comes back to the top, UIActivityIndicator hides part of the ListView.~~

☝️ this is caused by the reproducers that sets the refreshing to false and does not handle the state properly, so it is not a bug in Core.

cipolleschi avatar Apr 11 '24 09:04 cipolleschi

We found the cause of the issue, and it was because the RCTPullToRefreshViewComponentView was not recycled correctly. What happened is that the UIRefreshControl was not deallocated and remained assigned to a disposed ScrollView. By properly recreating it in prepare for recycle, we fixed the bug.

cipolleschi avatar Apr 11 '24 16:04 cipolleschi