perf(dashboard): Improve performance of complex dashboards (#19064)

* perf(dashboard): Improve performance of filter indicators * Improve perf of cross filters * Rename old function * Fix undefined * fix type * fix tests * fix undefined * Address code review comments * Address code review comments
apache · Mar 9, 2022 · 3c1fb94 · 3c1fb94
1 parent 220c461
commit 3c1fb94
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 51 deletions.
diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts
@@ -55,6 +55,7 @@ export const nativeFilters: NativeFiltersState = {
       },
       type: NativeFilterType.NATIVE_FILTER,
       description: '',
+      chartsInScope: [18],
     },
     'NATIVE_FILTER-x9QPw0so1': {
       id: 'NATIVE_FILTER-x9QPw0so1',
@@ -85,6 +86,7 @@ export const nativeFilters: NativeFiltersState = {
       },
       type: NativeFilterType.NATIVE_FILTER,
       description: '2 letter code',
+      chartsInScope: [18],
     },
   },
 };

diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
@@ -18,7 +18,7 @@
  */
 // ParentSize uses resize observer so the dashboard will update size
 // when its container size changes, due to e.g., builder side panel opening
-import React, { FC, useEffect, useState } from 'react';
+import React, { FC, useEffect, useMemo, useState } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import {
   FeatureFlag,
@@ -27,35 +27,55 @@ import {
   isFeatureEnabled,
 } from '@superset-ui/core';
 import { ParentSize } from '@vx/responsive';
+import pick from 'lodash/pick';
 import Tabs from 'src/components/Tabs';
 import DashboardGrid from 'src/dashboard/containers/DashboardGrid';
+import {
+  ChartsState,
+  DashboardLayout,
+  LayoutItem,
+  RootState,
+} from 'src/dashboard/types';
 import getLeafComponentIdFromPath from 'src/dashboard/util/getLeafComponentIdFromPath';
-import { DashboardLayout, LayoutItem, RootState } from 'src/dashboard/types';
 import {
   DASHBOARD_GRID_ID,
   DASHBOARD_ROOT_DEPTH,
 } from 'src/dashboard/util/constants';
+import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope';
+import findTabIndexByComponentId from 'src/dashboard/util/findTabIndexByComponentId';
+import { setInScopeStatusOfFilters } from 'src/dashboard/actions/nativeFilters';
 import { getRootLevelTabIndex, getRootLevelTabsComponent } from './utils';
-import { getChartIdsInFilterScope } from '../../util/activeDashboardFilters';
-import findTabIndexByComponentId from '../../util/findTabIndexByComponentId';
 import { findTabsWithChartsInScope } from '../nativeFilters/utils';
-import { setInScopeStatusOfFilters } from '../../actions/nativeFilters';
 import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils';
 
 type DashboardContainerProps = {
   topLevelTabs?: LayoutItem;
 };
 
+const useNativeFilterScopes = () => {
+  const nativeFilters = useSelector<RootState, Filters>(
+    state => state.nativeFilters?.filters,
+  );
+  return useMemo(
+    () =>
+      nativeFilters
+        ? Object.values(nativeFilters).map((filter: Filter) =>
+            pick(filter, ['id', 'scope', 'type']),
+          )
+        : [],
+    [JSON.stringify(nativeFilters)],
+  );
+};
+
 const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
   const dashboardLayout = useSelector<RootState, DashboardLayout>(
     state => state.dashboardLayout.present,
   );
-  const nativeFilters = useSelector<RootState, Filters>(
-    state => state.nativeFilters?.filters,
-  );
+  const nativeFilterScopes = useNativeFilterScopes();
   const directPathToChild = useSelector<RootState, string[]>(
     state => state.dashboardState.directPathToChild,
   );
+  const charts = useSelector<RootState, ChartsState>(state => state.charts);
   const [tabIndex, setTabIndex] = useState(
     getRootLevelTabIndex(dashboardLayout, directPathToChild),
   );
@@ -72,36 +92,26 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
     }
   }, [getLeafComponentIdFromPath(directPathToChild)]);
 
-  // recalculate charts and tabs in scopes of native filters only when a scope or dashboard layout changes
-  const filterScopes = Object.values(nativeFilters ?? {}).map(
-    (filter: Filter) => ({
-      id: filter.id,
-      scope: filter.scope,
-      type: filter.type,
-    }),
-  );
   useEffect(() => {
     if (
       !isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) ||
-      filterScopes.length === 0
+      nativeFilterScopes.length === 0
     ) {
       return;
     }
-    const scopes = filterScopes.map(filterScope => {
+    const scopes = nativeFilterScopes.map(filterScope => {
       if (filterScope.id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX)) {
         return {
           filterId: filterScope.id,
           tabsInScope: [],
           chartsInScope: [],
         };
       }
-      const { scope } = filterScope;
-      const chartsInScope: number[] = getChartIdsInFilterScope({
-        filterScope: {
-          scope: scope.rootPath,
-          immune: scope.excluded,
-        },
-      });
+      const chartsInScope: number[] = getChartIdsInFilterScope(
+        filterScope.scope,
+        charts,
+        dashboardLayout,
+      );
       const tabsInScope = findTabsWithChartsInScope(
         dashboardLayout,
         chartsInScope,
@@ -113,7 +123,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
       };
     });
     dispatch(setInScopeStatusOfFilters(scopes));
-  }, [JSON.stringify(filterScopes), dashboardLayout, dispatch]);
+  }, [nativeFilterScopes, dashboardLayout, dispatch]);
 
   const childIds: string[] = topLevelTabs
     ? topLevelTabs.children

diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts
@@ -27,11 +27,11 @@ import {
   NativeFilterType,
 } from '@superset-ui/core';
 import { NO_TIME_RANGE, TIME_FILTER_MAP } from 'src/explore/constants';
-import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters';
+import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters';
+import { CHART_TYPE } from 'src/dashboard/util/componentTypes';
 import { ChartConfiguration } from 'src/dashboard/reducers/types';
+import { Layout } from 'src/dashboard/types';
 import { areObjectsEqual } from 'src/reduxUtils';
-import { Layout } from '../../types';
-import { getTreeCheckedItems } from '../nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils';
 
 export enum IndicatorStatus {
   Unset = 'UNSET',
@@ -123,7 +123,7 @@ const selectIndicatorsForChartFromFilter = (
 
   return Object.keys(filter.columns)
     .filter(column =>
-      getChartIdsInFilterScope({
+      getChartIdsInFilterBoxScope({
         filterScope: filter.scopes[column],
       }).includes(chartId),
     )
@@ -274,10 +274,7 @@ export const selectNativeIndicatorsForChart = (
         .filter(
           nativeFilter =>
             nativeFilter.type === NativeFilterType.NATIVE_FILTER &&
-            getTreeCheckedItems(nativeFilter.scope, dashboardLayout).some(
-              layoutItem =>
-                dashboardLayout[layoutItem]?.meta?.chartId === chartId,
-            ),
+            nativeFilter.chartsInScope?.includes(chartId),
         )
         .map(nativeFilter => {
           const column = nativeFilter.targets?.[0]?.column?.name;
@@ -295,22 +292,27 @@ export const selectNativeIndicatorsForChart = (
 
   let crossFilterIndicators: any = [];
   if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
+    const dashboardLayoutValues = Object.values(dashboardLayout);
+    const chartLayoutItem = dashboardLayoutValues.find(
+      layoutItem => layoutItem?.meta?.chartId === chartId,
+    );
     crossFilterIndicators = Object.values(chartConfiguration)
-      .filter(chartConfig =>
-        getTreeCheckedItems(
-          chartConfig?.crossFilters?.scope,
-          dashboardLayout,
-        ).some(
-          layoutItem => dashboardLayout[layoutItem]?.meta?.chartId === chartId,
-        ),
+      .filter(
+        chartConfig =>
+          !chartConfig.crossFilters.scope.excluded.includes(chartId) &&
+          chartConfig.crossFilters.scope.rootPath.some(
+            elementId =>
+              chartLayoutItem?.type === CHART_TYPE &&
+              chartLayoutItem?.parents?.includes(elementId),
+          ),
       )
       .map(chartConfig => {
         const filterState = dataMask[chartConfig.id]?.filterState;
         const label = extractLabel(filterState);
         const filtersState = filterState?.filters;
         const column = filtersState && Object.keys(filtersState)[0];
 
-        const dashboardLayoutItem = Object.values(dashboardLayout).find(
+        const dashboardLayoutItem = dashboardLayoutValues.find(
           layoutItem => layoutItem?.meta?.chartId === chartConfig.id,
         );
         return {

diff --git a/superset-frontend/src/dashboard/components/filterscope/FilterScopeSelector.jsx b/superset-frontend/src/dashboard/components/filterscope/FilterScopeSelector.jsx
@@ -30,7 +30,7 @@ import getKeyForFilterScopeTree from 'src/dashboard/util/getKeyForFilterScopeTre
 import getSelectedChartIdForFilterScopeTree from 'src/dashboard/util/getSelectedChartIdForFilterScopeTree';
 import getFilterScopeFromNodesTree from 'src/dashboard/util/getFilterScopeFromNodesTree';
 import getRevertedFilterScope from 'src/dashboard/util/getRevertedFilterScope';
-import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters';
+import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters';
 import {
   getChartIdAndColumnFromFilterKey,
   getDashboardFilterKey,
@@ -106,7 +106,7 @@ export default class FilterScopeSelector extends React.PureComponent {
               const expanded = getFilterScopeParentNodes(nodes, 1);
               // force display filter_box chart as unchecked, but show checkbox as disabled
               const chartIdsInFilterScope = (
-                getChartIdsInFilterScope({
+                getChartIdsInFilterBoxScope({
                   filterScope: dashboardFilters[filterId].scopes[columnName],
                 }) || []
               ).filter(id => id !== filterId);

diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx
@@ -22,7 +22,7 @@ import cx from 'classnames';
 import { useTheme } from '@superset-ui/core';
 import { useSelector, connect } from 'react-redux';
 
-import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters';
+import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters';
 import Chart from '../../containers/Chart';
 import AnchorLink from '../../../components/AnchorLink';
 import DeleteComponentButton from '../DeleteComponentButton';
@@ -142,7 +142,7 @@ const FilterFocusHighlight = React.forwardRef(
       }
     } else if (
       chartId === focusedFilterScope.chartId ||
-      getChartIdsInFilterScope({
+      getChartIdsInFilterBoxScope({
         filterScope: focusedFilterScope.scope,
       }).includes(chartId)
     ) {

diff --git a/superset-frontend/src/dashboard/util/activeDashboardFilters.js b/superset-frontend/src/dashboard/util/activeDashboardFilters.js
@@ -18,7 +18,6 @@
  */
 import { isEmpty } from 'lodash';
 import { mapValues, flow, keyBy } from 'lodash/fp';
-
 import {
   getChartIdAndColumnFromFilterKey,
   getDashboardFilterKey,
@@ -62,7 +61,10 @@ export function getAppliedFilterValues(chartId) {
   return appliedFilterValuesByChart[chartId];
 }
 
-export function getChartIdsInFilterScope({ filterScope }) {
+// Legacy - getChartIdsInFilterBoxScope is used only by
+// components and functions related to filter box
+// Please use src/dashboard/util/getChartIdsInFilterScope instead
+export function getChartIdsInFilterBoxScope({ filterScope }) {
   function traverse(chartIds = [], component = {}, immuneChartIds = []) {
     if (!component) {
       return;
@@ -117,7 +119,7 @@ export function buildActiveFilters({ dashboardFilters = {}, components = {} }) {
           : columns[column] !== undefined
       ) {
         // remove filter itself
-        const scope = getChartIdsInFilterScope({
+        const scope = getChartIdsInFilterBoxScope({
           filterScope: scopes[column],
         }).filter(id => chartId !== id);
 

diff --git a/superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts b/superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts
@@ -26,7 +26,7 @@ import {
 } from 'src/explore/constants';
 import { DASHBOARD_FILTER_SCOPE_GLOBAL } from 'src/dashboard/reducers/dashboardFilters';
 import { Filter, NativeFilterType, TimeGranularity } from '@superset-ui/core';
-import { getChartIdsInFilterScope } from './activeDashboardFilters';
+import { getChartIdsInFilterBoxScope } from './activeDashboardFilters';
 import getFilterConfigsFromFormdata from './getFilterConfigsFromFormdata';
 
 interface FilterConfig {
@@ -147,7 +147,7 @@ const getFilterboxDependencies = (filterScopes: FilterScopesMetadata) => {
   Object.entries(filterScopes).forEach(([key, filterFields]) => {
     filterFieldsDependencies[key] = {};
     Object.entries(filterFields).forEach(([filterField, filterScope]) => {
-      filterFieldsDependencies[key][filterField] = getChartIdsInFilterScope({
+      filterFieldsDependencies[key][filterField] = getChartIdsInFilterBoxScope({
         filterScope,
       }).filter(
         chartId => filterChartIds.includes(chartId) && String(chartId) !== key,

diff --git a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { NativeFilterScope } from '@superset-ui/core';
+import { CHART_TYPE } from './componentTypes';
+import { ChartsState, Layout } from '../types';
+
+export function getChartIdsInFilterScope(
+  filterScope: NativeFilterScope,
+  charts: ChartsState,
+  layout: Layout,
+) {
+  const layoutItems = Object.values(layout);
+  return Object.values(charts)
+    .filter(
+      chart =>
+        !filterScope.excluded.includes(chart.id) &&
+        layoutItems
+          .find(
+            layoutItem =>
+              layoutItem?.type === CHART_TYPE &&
+              layoutItem.meta?.chartId === chart.id,
+          )
+          ?.parents?.some(elementId =>
+            filterScope.rootPath.includes(elementId),
+          ),
+    )
+    .map(chart => chart.id);
+}