[Discover] Improve sidebar performance by memoizing (#111843)

* Improve sidebar editField function by useCallback * Reduce re-renderings
2021-09-15 12:00:25 +02:00 · 2021-09-15 12:00:25 +02:00 · 13de9f598e
parent a949cc8121
commit 13de9f598e
5 changed files with 64 additions and 49 deletions
--- a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.test.tsx
+++ b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.test.tsx
@ -18,7 +18,7 @@ import { DiscoverSidebarProps } from './discover_sidebar';
 import { IndexPatternAttributes } from '../../../../../../../data/common';
 import { SavedObject } from '../../../../../../../../core/types';
 import { getDefaultFieldFilter } from './lib/field_filter';
-import { DiscoverSidebar } from './discover_sidebar';
+import { DiscoverSidebarComponent as DiscoverSidebar } from './discover_sidebar';
 import { ElasticSearchHit } from '../../../../doc_views/doc_views_types';
 import { discoverServiceMock as mockDiscoverServices } from '../../../../../__mocks__/services';
 import { stubLogstashIndexPattern } from '../../../../../../../data/common/stubs';
--- a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.tsx
+++ b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.tsx
@ -8,7 +8,7 @@

 import './discover_sidebar.scss';
 import { throttle } from 'lodash';
-import React, { useCallback, useEffect, useState, useMemo, useRef } from 'react';
+import React, { useCallback, useEffect, useState, useMemo, useRef, memo } from 'react';
 import { i18n } from '@kbn/i18n';
 import {
  EuiAccordion,
@ -69,14 +69,14 @@ export interface DiscoverSidebarProps extends Omit<DiscoverSidebarResponsiveProp
  /**
   * a statistics of the distribution of fields in the given hits
   */
-  fieldCounts: Record<string, number>;
+  fieldCounts?: Record<string, number>;
  /**
   * hits fetched from ES, displayed in the doc table
   */
-  documents: ElasticSearchHit[];
+  documents?: ElasticSearchHit[];
 }

-export function DiscoverSidebar({
+export function DiscoverSidebarComponent({
  alwaysShowActionButtons = false,
  columns,
  fieldCounts,
@ -109,8 +109,10 @@ export function DiscoverSidebar({
  const availableFieldsContainer = useRef<HTMLUListElement | null>(null);

  useEffect(() => {
-    const newFields = getIndexPatternFieldList(selectedIndexPattern, fieldCounts);
-    setFields(newFields);
+    if (documents) {
+      const newFields = getIndexPatternFieldList(selectedIndexPattern, fieldCounts);
+      setFields(newFields);
+    }
  }, [selectedIndexPattern, fieldCounts, documents]);

  const scrollDimensions = useResizeObserver(scrollContainer);
@ -265,7 +267,7 @@ export function DiscoverSidebar({

  const filterChanged = useMemo(() => isEqual(fieldFilter, getDefaultFieldFilter()), [fieldFilter]);

-  if (!selectedIndexPattern || !fields) {
+  if (!selectedIndexPattern) {
    return null;
  }

@ -344,7 +346,7 @@ export function DiscoverSidebar({
        <EuiFlexItem className="eui-yScroll">
          <div
            ref={(el) => {
-              if (el && !el.dataset.dynamicScroll) {
+              if (documents && el && !el.dataset.dynamicScroll) {
                el.dataset.dynamicScroll = 'true';
                setScrollContainer(el);
              }
@ -352,7 +354,7 @@ export function DiscoverSidebar({
            onScroll={throttle(lazyScroll, 100)}
            className="eui-yScroll"
          >
-            {fields.length > 0 && (
+            {Array.isArray(fields) && fields.length > 0 && (
              <div>
                {selectedFields &&
                selectedFields.length > 0 &&
@ -500,3 +502,5 @@ export function DiscoverSidebar({
    </EuiPageSideBar>
  );
 }
+
+export const DiscoverSidebar = memo(DiscoverSidebarComponent);
--- a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.tsx
+++ b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.tsx
@ -114,6 +114,7 @@ export interface DiscoverSidebarResponsiveProps {
 * Mobile: Index pattern selector is visible and a button to trigger a flyout with all elements
 */
 export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps) {
+  const { selectedIndexPattern, onEditRuntimeField, useNewFieldsApi, onChangeIndexPattern } = props;
  const [fieldFilter, setFieldFilter] = useState(getDefaultFieldFilter());
  const [isFlyoutVisible, setIsFlyoutVisible] = useState(false);
  /**
@ -125,7 +126,7 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps)
    fieldCounts.current = calcFieldCounts(
      {},
      props.documents$.getValue().result,
-      props.selectedIndexPattern
+      selectedIndexPattern
    );
  }

@ -137,20 +138,20 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps)
          fieldCounts.current = calcFieldCounts(
            next.result.length && fieldCounts.current ? fieldCounts.current : {},
            next.result,
-            props.selectedIndexPattern!
+            selectedIndexPattern!
          );
        }
        setDocumentState({ ...documentState, ...next });
      }
    });
    return () => subscription.unsubscribe();
-  }, [props.documents$, props.selectedIndexPattern, documentState, setDocumentState]);
+  }, [props.documents$, selectedIndexPattern, documentState, setDocumentState]);

  useEffect(() => {
    // when index pattern changes fieldCounts needs to be cleaned up to prevent displaying
    // fields of the previous index pattern
    fieldCounts.current = {};
-  }, [props.selectedIndexPattern]);
+  }, [selectedIndexPattern]);

  const closeFieldEditor = useRef<() => void | undefined>();

@ -174,42 +175,52 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps)
    setIsFlyoutVisible(false);
  }, []);

-  if (!props.selectedIndexPattern) {
+  const { indexPatternFieldEditor } = props.services;
+
+  const editField = useCallback(
+    (fieldName?: string) => {
+      const indexPatternFieldEditPermission = indexPatternFieldEditor?.userPermissions.editIndexPattern();
+      const canEditIndexPatternField = !!indexPatternFieldEditPermission && useNewFieldsApi;
+      if (!canEditIndexPatternField || !selectedIndexPattern) {
+        return;
+      }
+      const ref = indexPatternFieldEditor.openEditor({
+        ctx: {
+          indexPattern: selectedIndexPattern,
+        },
+        fieldName,
+        onSave: async () => {
+          onEditRuntimeField();
+        },
+      });
+      if (setFieldEditorRef) {
+        setFieldEditorRef(ref);
+      }
+      if (closeFlyout) {
+        closeFlyout();
+      }
+    },
+    [
+      closeFlyout,
+      indexPatternFieldEditor,
+      selectedIndexPattern,
+      setFieldEditorRef,
+      onEditRuntimeField,
+      useNewFieldsApi,
+    ]
+  );
+
+  if (!selectedIndexPattern) {
    return null;
  }

-  const { indexPatternFieldEditor } = props.services;
-  const indexPatternFieldEditPermission = indexPatternFieldEditor?.userPermissions.editIndexPattern();
-  const canEditIndexPatternField = !!indexPatternFieldEditPermission && props.useNewFieldsApi;
-
-  const editField = (fieldName?: string) => {
-    if (!canEditIndexPatternField || !props.selectedIndexPattern) {
-      return;
-    }
-    const ref = indexPatternFieldEditor.openEditor({
-      ctx: {
-        indexPattern: props.selectedIndexPattern,
-      },
-      fieldName,
-      onSave: async () => {
-        props.onEditRuntimeField();
-      },
-    });
-    if (setFieldEditorRef) {
-      setFieldEditorRef(ref);
-    }
-    if (closeFlyout) {
-      closeFlyout();
-    }
-  };
-
  return (
    <>
      {props.isClosed ? null : (
        <EuiHideFor sizes={['xs', 's']}>
          <DiscoverSidebar
            {...props}
-            documents={documentState.result ?? []}
+            documents={documentState.result}
            fieldFilter={fieldFilter}
            fieldCounts={fieldCounts.current}
            setFieldFilter={setFieldFilter}
@ -230,17 +241,17 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps)
            <EuiFlexGroup direction="row" gutterSize="s" alignItems="center" responsive={false}>
              <EuiFlexItem grow={true}>
                <DiscoverIndexPattern
-                  onChangeIndexPattern={props.onChangeIndexPattern}
-                  selectedIndexPattern={props.selectedIndexPattern}
+                  onChangeIndexPattern={onChangeIndexPattern}
+                  selectedIndexPattern={selectedIndexPattern}
                  indexPatternList={sortBy(props.indexPatternList, (o) => o.attributes.title)}
                />
              </EuiFlexItem>
              <EuiFlexItem grow={false}>
                <DiscoverIndexPatternManagement
                  services={props.services}
-                  selectedIndexPattern={props.selectedIndexPattern}
+                  selectedIndexPattern={selectedIndexPattern}
                  editField={editField}
-                  useNewFieldsApi={props.useNewFieldsApi}
+                  useNewFieldsApi={useNewFieldsApi}
                />
              </EuiFlexItem>
            </EuiFlexGroup>
@ -296,7 +307,7 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps)
              <div className="euiFlyoutBody">
                <DiscoverSidebar
                  {...props}
-                  documents={documentState.result ?? []}
+                  documents={documentState.result}
                  fieldCounts={fieldCounts.current}
                  fieldFilter={fieldFilter}
                  setFieldFilter={setFieldFilter}
--- a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/get_details.ts
+++ b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/get_details.ts
@ -13,11 +13,11 @@ import { ElasticSearchHit } from '../../../../../doc_views/doc_views_types';

 export function getDetails(
  field: IndexPatternField,
-  hits: ElasticSearchHit[],
+  hits: ElasticSearchHit[] | undefined,
  columns: string[],
  indexPattern?: IndexPattern
 ) {
-  if (!indexPattern) {
+  if (!indexPattern || !hits) {
    return {};
  }
  const details = {
--- a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.tsx
+++ b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/group_fields.tsx
@ -22,7 +22,7 @@ export function groupFields(
  fields: IndexPatternField[] | null,
  columns: string[],
  popularLimit: number,
-  fieldCounts: Record<string, number>,
+  fieldCounts: Record<string, number> | undefined,
  fieldFilterState: FieldFilterState,
  useNewFieldsApi: boolean
 ): GroupedFields {