Merge pull request #31711 from TMisiukiewicz/perf/lhn-flashlist-migra…

…tion [perf] Migrate LHNOptionList to FlashList
Expensify · Nov 22, 2023 · 881b511 · 881b511
2 parents a273354 + e18243f
commit 881b511
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 50 deletions.
diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js
@@ -1,7 +1,8 @@
+import {FlashList} from '@shopify/flash-list';
 import lodashGet from 'lodash/get';
 import PropTypes from 'prop-types';
 import React, {useCallback} from 'react';
-import {FlatList, View} from 'react-native';
+import {View} from 'react-native';
 import {withOnyx} from 'react-native-onyx';
 import _ from 'underscore';
 import participantPropTypes from '@components/participantPropTypes';
@@ -11,6 +12,7 @@ import compose from '@libs/compose';
 import * as OptionsListUtils from '@libs/OptionsListUtils';
 import reportActionPropTypes from '@pages/home/report/reportActionPropTypes';
 import reportPropTypes from '@pages/reportPropTypes';
+import stylePropTypes from '@styles/stylePropTypes';
 import useThemeStyles from '@styles/useThemeStyles';
 import variables from '@styles/variables';
 import CONST from '@src/CONST';
@@ -19,12 +21,10 @@ import OptionRowLHNData from './OptionRowLHNData';
 
 const propTypes = {
     /** Wrapper style for the section list */
-    // eslint-disable-next-line react/forbid-prop-types
-    style: PropTypes.arrayOf(PropTypes.object),
+    style: stylePropTypes,
 
     /** Extra styles for the section list container */
-    // eslint-disable-next-line react/forbid-prop-types
-    contentContainerStyles: PropTypes.arrayOf(PropTypes.object).isRequired,
+    contentContainerStyles: stylePropTypes.isRequired,
 
     /** Sections for the section list */
     data: PropTypes.arrayOf(PropTypes.string).isRequired,
@@ -80,7 +80,7 @@ const defaultProps = {
     ...withCurrentReportIDDefaultProps,
 };
 
-const keyExtractor = (item) => item;
+const keyExtractor = (item) => `report_${item}`;
 
 function LHNOptionsList({
     style,
@@ -99,28 +99,6 @@ function LHNOptionsList({
     currentReportID,
 }) {
     const styles = useThemeStyles();
-    /**
-     * This function is used to compute the layout of any given item in our list. Since we know that each item will have the exact same height, this is a performance optimization
-     * so that the heights can be determined before the options are rendered. Otherwise, the heights are determined when each option is rendering and it causes a lot of overhead on large
-     * lists.
-     *
-     * @param {Array} itemData - This is the same as the data we pass into the component
-     * @param {Number} index the current item's index in the set of data
-     *
-     * @returns {Object}
-     */
-    const getItemLayout = useCallback(
-        (itemData, index) => {
-            const optionHeight = optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight;
-            return {
-                length: optionHeight,
-                offset: index * optionHeight,
-                index,
-            };
-        },
-        [optionMode],
-    );
-
     /**
      * Function which renders a row in the list
      *
@@ -164,20 +142,17 @@ function LHNOptionsList({
 
     return (
         <View style={style || styles.flex1}>
-            <FlatList
+            <FlashList
                 indicatorStyle="white"
                 keyboardShouldPersistTaps="always"
                 contentContainerStyle={contentContainerStyles}
-                showsVerticalScrollIndicator={false}
                 data={data}
                 testID="lhn-options-list"
                 keyExtractor={keyExtractor}
-                stickySectionHeadersEnabled={false}
                 renderItem={renderItem}
-                getItemLayout={getItemLayout}
-                initialNumToRender={20}
-                maxToRenderPerBatch={5}
-                windowSize={5}
+                estimatedItemSize={optionMode === CONST.OPTION_MODE.COMPACT ? variables.optionRowHeightCompact : variables.optionRowHeight}
+                extraData={[currentReportID]}
+                showsVerticalScrollIndicator={false}
             />
         </View>
     );

diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js
@@ -1,7 +1,7 @@
 /* eslint-disable rulesdir/onyx-props-must-have-default */
 import PropTypes from 'prop-types';
 import React, {useCallback, useEffect, useRef} from 'react';
-import {InteractionManager, View} from 'react-native';
+import {InteractionManager, StyleSheet, View} from 'react-native';
 import _ from 'underscore';
 import LogoComponent from '@assets/images/expensify-wordmark.svg';
 import Header from '@components/Header';
@@ -177,16 +177,21 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority
                 </Tooltip>
                 <SignInOrAvatarWithOptionalStatus isCreateMenuOpen={isCreateMenuOpen} />
             </View>
-
-            <LHNOptionsList
-                style={[isLoading ? styles.flexShrink1 : styles.flex1]}
-                contentContainerStyles={[styles.sidebarListContainer, {paddingBottom: StyleUtils.getSafeAreaMargins(insets).marginBottom}]}
-                data={optionListItems}
-                onSelectRow={showReportPage}
-                shouldDisableFocusOptions={isSmallScreenWidth}
-                optionMode={viewMode}
-            />
-            {isLoading && <OptionsListSkeletonView shouldAnimate />}
+            <View style={[styles.pRelative, styles.flex1]}>
+                <LHNOptionsList
+                    style={styles.flex1}
+                    contentContainerStyles={StyleSheet.flatten([styles.sidebarListContainer, {paddingBottom: StyleUtils.getSafeAreaMargins(insets).marginBottom}])}
+                    data={optionListItems}
+                    onSelectRow={showReportPage}
+                    shouldDisableFocusOptions={isSmallScreenWidth}
+                    optionMode={viewMode}
+                />
+                {isLoading && optionListItems.length === 0 && (
+                    <View style={[StyleSheet.absoluteFillObject, styles.highlightBG]}>
+                        <OptionsListSkeletonView shouldAnimate />
+                    </View>
+                )}
+            </View>
         </View>
     );
 }

diff --git a/src/styles/styles.ts b/src/styles/styles.ts
@@ -1367,7 +1367,6 @@ const styles = (theme: ThemeColors) =>
         },
 
         sidebarListContainer: {
-            scrollbarWidth: 'none',
             paddingBottom: 4,
         },
 

diff --git a/tests/perf-test/SidebarLinks.perf-test.js b/tests/perf-test/SidebarLinks.perf-test.js
@@ -99,9 +99,9 @@ test('should scroll and click some of the items', () => {
         const lhnOptionsList = await screen.findByTestId('lhn-options-list');
 
         fireEvent.scroll(lhnOptionsList, eventData);
-
-        const button1 = await screen.findByTestId('1');
-        const button2 = await screen.findByTestId('2');
+        // find elements that are currently visible in the viewport
+        const button1 = await screen.findByTestId('7');
+        const button2 = await screen.findByTestId('8');
         fireEvent.press(button1);
         fireEvent.press(button2);
     };
-Original file line number
+Diff line change
@@ Expand Up / @@ -1367,7 +1367,6 @@ const styles = (theme: ThemeColors) => @@
             },
             sidebarListContainer: {
-                scrollbarWidth: 'none',
                 paddingBottom: 4,
             },
@@ Expand Down @@