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
2022-03-09 22:39:58 +01:00 · 2022-03-09 22:39:58 +01:00 · 3c1fb944c1
parent 220c46131e
commit 3c1fb944c1
8 changed files with 111 additions and 51 deletions
--- 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],
    },
  },
 };
--- 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,22 +92,14 @@ 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,
@ -95,13 +107,11 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
          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
--- 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,14 +292,19 @@ 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;
@ -310,7 +312,7 @@ export const selectNativeIndicatorsForChart = (
        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 {
--- 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);
--- 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)
    ) {
--- 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);

--- 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,
--- 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);
+}