[Lens] More memoization work on the editor_frame (elastic#102186) (el…

…astic#102505) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
kibanamachine · Jun 17, 2021 · bdcc729 · bdcc729
1 parent 24f51bf
commit bdcc729
Show file tree

Hide file tree

Showing 6 changed files with 168 additions and 137 deletions.
diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx
@@ -70,6 +70,9 @@ export function LayerPanel(
     updateDatasource,
     toggleFullscreen,
     isFullscreen,
+    updateAll,
+    updateDatasourceAsync,
+    visualizationState,
   } = props;
   const datasourcePublicAPI = framePublicAPI.datasourceLayers[layerId];
 
@@ -114,7 +117,11 @@ export function LayerPanel(
     activeData: props.framePublicAPI.activeData,
   };
 
-  const { groups } = activeVisualization.getConfiguration(layerVisualizationConfigProps);
+  const { groups } = useMemo(
+    () => activeVisualization.getConfiguration(layerVisualizationConfigProps),
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [layerDatasourceConfigProps.frame, layerDatasourceDropProps, activeVisualization]
+  );
   const isEmptyLayer = !groups.some((d) => d.accessors.length > 0);
   const { activeId, activeGroup } = activeDimension;
 
@@ -204,6 +211,58 @@ export function LayerPanel(
 
   const isDimensionPanelOpen = Boolean(activeId);
 
+  const updateDataLayerState = useCallback(
+    (newState: unknown, { isDimensionComplete = true }: { isDimensionComplete?: boolean } = {}) => {
+      if (!activeGroup || !activeId) {
+        return;
+      }
+      if (allAccessors.includes(activeId)) {
+        if (isDimensionComplete) {
+          updateDatasourceAsync(datasourceId, newState);
+        } else {
+          // The datasource can indicate that the previously-valid column is no longer
+          // complete, which clears the visualization. This keeps the flyout open and reuses
+          // the previous columnId
+          updateAll(
+            datasourceId,
+            newState,
+            activeVisualization.removeDimension({
+              layerId,
+              columnId: activeId,
+              prevState: visualizationState,
+            })
+          );
+        }
+      } else if (isDimensionComplete) {
+        updateAll(
+          datasourceId,
+          newState,
+          activeVisualization.setDimension({
+            layerId,
+            groupId: activeGroup.groupId,
+            columnId: activeId,
+            prevState: visualizationState,
+          })
+        );
+        setActiveDimension({ ...activeDimension, isNew: false });
+      } else {
+        updateDatasourceAsync(datasourceId, newState);
+      }
+    },
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [
+      activeDimension,
+      activeGroup,
+      activeId,
+      activeVisualization,
+      datasourceId,
+      layerId,
+      updateAll,
+      updateDatasourceAsync,
+      visualizationState,
+    ]
+  );
+
   return (
     <>
       <section
@@ -460,43 +519,7 @@ export function LayerPanel(
                   dimensionGroups: groups,
                   toggleFullscreen,
                   isFullscreen,
-                  setState: (
-                    newState: unknown,
-                    { isDimensionComplete = true }: { isDimensionComplete?: boolean } = {}
-                  ) => {
-                    if (allAccessors.includes(activeId)) {
-                      if (isDimensionComplete) {
-                        props.updateDatasourceAsync(datasourceId, newState);
-                      } else {
-                        // The datasource can indicate that the previously-valid column is no longer
-                        // complete, which clears the visualization. This keeps the flyout open and reuses
-                        // the previous columnId
-                        props.updateAll(
-                          datasourceId,
-                          newState,
-                          activeVisualization.removeDimension({
-                            layerId,
-                            columnId: activeId,
-                            prevState: props.visualizationState,
-                          })
-                        );
-                      }
-                    } else if (isDimensionComplete) {
-                      props.updateAll(
-                        datasourceId,
-                        newState,
-                        activeVisualization.setDimension({
-                          layerId,
-                          groupId: activeGroup.groupId,
-                          columnId: activeId,
-                          prevState: props.visualizationState,
-                        })
-                      );
-                      setActiveDimension({ ...activeDimension, isNew: false });
-                    } else {
-                      props.updateDatasourceAsync(datasourceId, newState);
-                    }
-                  },
+                  setState: updateDataLayerState,
                 }}
               />
             )}

diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx
@@ -570,9 +570,8 @@ describe('editor_frame', () => {
         setDatasourceState(updatedState);
       });
 
-      // TODO: temporary regression
       // validation requires to calls this getConfiguration API
-      expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(9);
+      expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(6);
       expect(mockVisualization.getConfiguration).toHaveBeenLastCalledWith(
         expect.objectContaining({
           state: updatedState,
@@ -645,9 +644,8 @@ describe('editor_frame', () => {
         setDatasourceState({});
       });
 
-      // TODO: temporary regression, selectors will help
       // validation requires to calls this getConfiguration API
-      expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(9);
+      expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(6);
       expect(mockVisualization.getConfiguration).toHaveBeenLastCalledWith(
         expect.objectContaining({
           frame: expect.objectContaining({
@@ -1124,8 +1122,7 @@ describe('editor_frame', () => {
       });
 
       // validation requires to calls this getConfiguration API
-      // TODO: why so many times?
-      expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(10);
+      expect(mockVisualization.getConfiguration).toHaveBeenCalledTimes(7);
       expect(mockVisualization.getConfiguration).toHaveBeenCalledWith(
         expect.objectContaining({
           state: suggestionVisState,

diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import React, { useEffect, useReducer, useState, useCallback, useRef } from 'react';
+import React, { useEffect, useReducer, useState, useCallback, useRef, useMemo } from 'react';
 import { CoreStart } from 'kibana/public';
 import { isEqual } from 'lodash';
 import { PaletteRegistry } from 'src/plugins/charts/public';
@@ -118,56 +118,74 @@ export function EditorFrame(props: EditorFrameProps) {
   );
   const datasourceLayers = createDatasourceLayers(props.datasourceMap, state.datasourceStates);
 
-  const framePublicAPI: FramePublicAPI = {
-    datasourceLayers,
-    activeData,
-    dateRange,
-    query,
-    filters,
-    searchSessionId,
-    availablePalettes: props.palettes,
-
-    addNewLayer() {
-      const newLayerId = generateId();
-
-      dispatch({
-        type: 'UPDATE_LAYER',
-        datasourceId: state.activeDatasourceId!,
-        layerId: newLayerId,
-        updater: props.datasourceMap[state.activeDatasourceId!].insertLayer,
-      });
+  const framePublicAPI: FramePublicAPI = useMemo(
+    () => ({
+      datasourceLayers,
+      activeData,
+      dateRange,
+      query,
+      filters,
+      searchSessionId,
+      availablePalettes: props.palettes,
 
-      return newLayerId;
-    },
+      addNewLayer() {
+        const newLayerId = generateId();
 
-    removeLayers(layerIds: string[]) {
-      if (activeVisualization && activeVisualization.removeLayer && state.visualization.state) {
         dispatch({
-          type: 'UPDATE_VISUALIZATION_STATE',
-          visualizationId: activeVisualization.id,
-          updater: layerIds.reduce(
-            (acc, layerId) =>
-              activeVisualization.removeLayer ? activeVisualization.removeLayer(acc, layerId) : acc,
-            state.visualization.state
-          ),
+          type: 'UPDATE_LAYER',
+          datasourceId: state.activeDatasourceId!,
+          layerId: newLayerId,
+          updater: props.datasourceMap[state.activeDatasourceId!].insertLayer,
         });
-      }
 
-      layerIds.forEach((layerId) => {
-        const layerDatasourceId = Object.entries(props.datasourceMap).find(
-          ([datasourceId, datasource]) =>
-            state.datasourceStates[datasourceId] &&
-            datasource.getLayers(state.datasourceStates[datasourceId].state).includes(layerId)
-        )![0];
-        dispatch({
-          type: 'UPDATE_LAYER',
-          layerId,
-          datasourceId: layerDatasourceId,
-          updater: props.datasourceMap[layerDatasourceId].removeLayer,
+        return newLayerId;
+      },
+
+      removeLayers(layerIds: string[]) {
+        if (activeVisualization && activeVisualization.removeLayer && state.visualization.state) {
+          dispatch({
+            type: 'UPDATE_VISUALIZATION_STATE',
+            visualizationId: activeVisualization.id,
+            updater: layerIds.reduce(
+              (acc, layerId) =>
+                activeVisualization.removeLayer
+                  ? activeVisualization.removeLayer(acc, layerId)
+                  : acc,
+              state.visualization.state
+            ),
+          });
+        }
+
+        layerIds.forEach((layerId) => {
+          const layerDatasourceId = Object.entries(props.datasourceMap).find(
+            ([datasourceId, datasource]) =>
+              state.datasourceStates[datasourceId] &&
+              datasource.getLayers(state.datasourceStates[datasourceId].state).includes(layerId)
+          )![0];
+          dispatch({
+            type: 'UPDATE_LAYER',
+            layerId,
+            datasourceId: layerDatasourceId,
+            updater: props.datasourceMap[layerDatasourceId].removeLayer,
+          });
         });
-      });
-    },
-  };
+      },
+    }),
+    [
+      activeData,
+      activeVisualization,
+      datasourceLayers,
+      dateRange,
+      query,
+      filters,
+      searchSessionId,
+      props.palettes,
+      props.datasourceMap,
+      state.activeDatasourceId,
+      state.datasourceStates,
+      state.visualization.state,
+    ]
+  );
 
   useEffect(
     () => {

diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts
@@ -7,6 +7,7 @@
 
 import { SavedObjectReference } from 'kibana/public';
 import { Ast } from '@kbn/interpreter/common';
+import memoizeOne from 'memoize-one';
 import {
   Datasource,
   DatasourcePublicAPI,
@@ -53,7 +54,7 @@ export async function initializeDatasources(
   return states;
 }
 
-export function createDatasourceLayers(
+export const createDatasourceLayers = memoizeOne(function createDatasourceLayers(
   datasourceMap: Record<string, Datasource>,
   datasourceStates: Record<string, { state: unknown; isLoading: boolean }>
 ) {
@@ -73,7 +74,7 @@ export function createDatasourceLayers(
       });
     });
   return datasourceLayers;
-}
+});
 
 export async function persistedStateToExpression(
   datasources: Record<string, Datasource>,