akveo / react-native-ui-kitten

:boom: React Native UI Library based on Eva Design System :new_moon_with_face::sparkles:Dark Mode
https://akveo.github.io/react-native-ui-kitten/
MIT License
10.3k stars 952 forks source link

List manipulation slow component performance #1276

Open Bandwagoner opened 3 years ago

Bandwagoner commented 3 years ago

🐛 Bug Report

Doing any type of manipulation on List component, for example adding new list items or re-ordering, produces slow re-rendering performance regardless of the number of elements in the list. Takes about 0,5 seconds for the list to re-render with the new updates.

To Reproduce

Created a List that renders ListItems derived from an array in component state. Create a button that adds a new item on to the array and sets state. When pressing the button, List should re-render with the new item almost immediately.

Expected behavior

Re-rendering of the list should be snappy.

Link to runnable example or repository (highly encouraged)

https://snack.expo.io/iwdW_iLw4

UI Kitten and Eva version

Package Version
@eva-design/eva 5.0.0
@ui-kitten/components 2.0.0

Environment information

System: OS: Windows 10 10.0.18363 CPU: (4) x64 AMD Ryzen 3 2300X Quad-Core Processor Binaries: Node: 12.19.0 - C:\Program Files\nodejs\node.EXE npm: 6.14.8 - C:\Program Files\nodejs\npm.CMD IDEs: Android Studio: Version 4.1.0.0 AI-201.8743.12.41.6858069 npmPackages: react: 16.14.0 => 16.14.0 react-native: 0.63.3 => 0.63.3

CostachescuCristinel commented 3 years ago

I don't think this is UI Kitten's issue. You are using hooks, which is fine, but in your hook functions you are constantly re-creating your elements using code like this:

  const renderThemeIcon = (props) => (
    <Icon {...props} name={theming.isDark ? 'sun' : 'moon'} />
  );

  const renderDeleteIcon = (props: any) => (
    <Icon {...props} name="trash-2-outline" />
  );

  const renderAddButton = () => {
    return (
      <Button onPress={onAdd}>
        <Text>Add new</Text>
      </Button>
    );
  };

The result of this is that on every render react will create a new constant, with a new arrow function, that returns a new component. This means that you are re-rendering from scratch every component that is rendered like this.

You are also using splice to remove an element from an array. Array.splice() modifies the original array, which tells React that the array of items is still the same, and will not re-render it. This is equivalent of manually modifying the state in a class component (this.state.items[0] = "new value";). The only reason you see items being removed from the array, is because of the side effect of re-creating components on each re-render (and the re-render is triggered by something else, not by your state-changing actions);

This causes performance issue within React, not within UI Kitten.

While the case above is for components, the same is applicable for your action functions:

  const toggle = () => {
    const nextThemeName = theme === 'light' ? 'dark' : 'light';
    setTheme(nextThemeName);
  };

For the cases above with the icons, the solution is to place the icon rendering functions outside the component's render:

const renderThemeIconLight = (props) => (
    <Icon {...props} name='sun' />
  );

const renderThemeIconDark = (props) => (
    <Icon {...props} name='moon' />
  );

  const renderDeleteIcon = (props: any) => (
    <Icon {...props} name="trash-2-outline" />
  );

// ...

export default ()=>{
  // ...
  const theming = useEvaTheming();
  const renderThemeIcon = (theming.isDark === true)
    ? renderThemeIconDark
    : renderThemeIconLight;

  return (
    /* ... */
       <Button
            appearance="ghost"
            accessoryLeft={renderDeleteIcon}
            onPress={() => onDelete(info.index)} // this is also re-recreated on each re-render
        />
    /* ... */
  );
}

For anything else where you need to pass other arguments for the functions that you call (such as the onDelete function above), a class component is the only solution (see note 1)

const renderThemeIconLight = (props) => (
    <Icon {...props} name='sun />
  );

const renderThemeIconDark = (props) => (
    <Icon {...props} name='moon' />
  );

  const renderDeleteIcon = (props: any) => (
    <Icon {...props} name="trash-2-outline" />
  );

export default class extends React.PureComponent {
    state = { items = [] };

    onDelete = ()=>{
        // methods on a class are only created (instantiated) once at mount time
        // They will not be re-created on each re-render

        // newItems.splice(index, 1); // do not do this! Create a new array using other ways

        const newItems = this.state.items.filter((item, index)=>{
            // Array.filter creates a new array with only the elemens that pass the test (this functions returns true)
            return (index !== 0); // removes the first item in the array
        });
        this.setState({ items: newItems });
    }

    renderThemeIcon = ()=>{
        // Do not make the mistake of thinking that a class, or class method solves all issues!
        // Returning a new function here like ()=><Icon ... />  is the same as how it was done in your original code with hooks
        // Instead, leave the icon rendering functions outside the class, and only return their reference from here
        // You can also use this code into the render method without creating this separate method function for it, but for the sake of an example... here it is

        // since you cannot use hooks in a class component, you must find another way of retrieving the theme
        const theme = { isDark: true };

        return (theming.isDark === true) ? renderThemeIconDark : renderThemeIconLight
    }

    render = () => {
        return <Button
            appearance="ghost"
            accessoryLeft={this.renderThemeIcon()}
            onPress={this.onDelete}
        />;
    }
};

note 1: You could use React.memo which aims at reusing a previously evaluated result if data did not change, but adds some complexity to the code, and I also see it as a hack-way of dealing with the original issue

I highly recomment reading about immutability in React, how to avoid reconciliation issues, and following tutorials on when and how to use class components, function components, or hooks.

Bandwagoner commented 3 years ago

You are correct on the memoization front. In my actual code I am using React.useCallback on the icon render functions.

On another note, I discovered that performance was related to the Select component. I am rendering a Select in every row. I migrated to React Native Picker and there no longer was this issue.

nbaua commented 3 years ago

Even without any manipulation within the list item, the list rendering is slow. The change in theme when the list is bound to a large array takes about 2-3 seconds to reflect on the simulator. I've not tested this on the real physical device yet, however I am sure the behavior should be more or less same on the device too.

I also encountered VirtualizedList: You have a large list that is slow to update issue (which is a common issue with FlatList control, provided by react native team), while the API says Performant interface for rendering simple, flat lists., I'd rather expect the component should have taken care of the virtualization for me and abstract only the required parameters to be passed (plus my item rendering logic).