fix(native filters): rendering performance improvement by reduce overrendering (#25901)

superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts

@@ -58,7 +58,6 @@ export enum AppSection {
 export type FilterState = { value?: any; [key: string]: any };
 
 export type DataMask = {
-  __cache?: FilterState;
   extraFormData?: ExtraFormData;
   filterState?: FilterState;
   ownState?: JsonObject;

superset-frontend/src/dashboard/components/Dashboard.jsx

@@ -25,9 +25,8 @@ import Loading from 'src/components/Loading';
 import getBootstrapData from 'src/utils/getBootstrapData';
 import getChartIdsFromLayout from '../util/getChartIdsFromLayout';
 import getLayoutComponentFromChartId from '../util/getLayoutComponentFromChartId';
-import DashboardBuilder from './DashboardBuilder/DashboardBuilder';
+
 import {
-  chartPropShape,
   slicePropShape,
   dashboardInfoPropShape,
   dashboardStatePropShape,
@@ -53,7 +52,6 @@ const propTypes = {
   }).isRequired,
   dashboardInfo: dashboardInfoPropShape.isRequired,
   dashboardState: dashboardStatePropShape.isRequired,
-  charts: PropTypes.objectOf(chartPropShape).isRequired,
   slices: PropTypes.objectOf(slicePropShape).isRequired,
   activeFilters: PropTypes.object.isRequired,
   chartConfiguration: PropTypes.object,
@@ -213,11 +211,6 @@ class Dashboard extends React.PureComponent {
     }
   }
 
-  // return charts in array
-  getAllCharts() {
-    return Object.values(this.props.charts);
-  }
-
   applyFilters() {
     const { appliedFilters } = this;
     const { activeFilters, ownDataCharts } = this.props;
@@ -288,11 +281,7 @@ class Dashboard extends React.PureComponent {
     if (this.context.loading) {
       return <Loading />;
     }
-    return (
-      <>
-        <DashboardBuilder />
-      </>
-    );
+    return this.props.children;
   }
 }
 

superset-frontend/src/dashboard/components/Dashboard.test.jsx

@@ -21,7 +21,6 @@ import { shallow } from 'enzyme';
 import sinon from 'sinon';
 
 import Dashboard from 'src/dashboard/components/Dashboard';
-import DashboardBuilder from 'src/dashboard/components/DashboardBuilder/DashboardBuilder';
 import { CHART_TYPE } from 'src/dashboard/util/componentTypes';
 import newComponentFactory from 'src/dashboard/util/newComponentFactory';
 
@@ -63,8 +62,14 @@ describe('Dashboard', () => {
     loadStats: {},
   };
 
+  const ChildrenComponent = () => <div>Test</div>;
+
   function setup(overrideProps) {
-    const wrapper = shallow(<Dashboard {...props} {...overrideProps} />);
+    const wrapper = shallow(
+      <Dashboard {...props} {...overrideProps}>
+        <ChildrenComponent />
+      </Dashboard>,
+    );
     return wrapper;
   }
 
@@ -76,9 +81,9 @@ describe('Dashboard', () => {
     '3_country_name': { values: ['USA'], scope: [] },
   };
 
-  it('should render a DashboardBuilder', () => {
+  it('should render the children component', () => {
     const wrapper = setup();
-    expect(wrapper.find(DashboardBuilder)).toExist();
+    expect(wrapper.find(ChildrenComponent)).toExist();
   });
 
   describe('UNSAFE_componentWillReceiveProps', () => {

superset-frontend/src/dashboard/components/SyncDashboardState/SyncDashboardState.test.tsx

@@ -0,0 +1,34 @@
+/**
+ * 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 React from 'react';
+import { render } from 'spec/helpers/testing-library';
+import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers';
+import SyncDashboardState from '.';
+
+test('stores the dashboard info with local storages', () => {
+  const testDashboardPageId = 'dashboardPageId';
+  render(<SyncDashboardState dashboardPageId={testDashboardPageId} />, {
+    useRedux: true,
+  });
+  expect(getItem(LocalStorageKeys.dashboard__explore_context, {})).toEqual({
+    [testDashboardPageId]: expect.objectContaining({
+      dashboardPageId: testDashboardPageId,
+    }),
+  });
+});

superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx

@@ -0,0 +1,103 @@
+/**
+ * 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 React, { useEffect } from 'react';
+import pick from 'lodash/pick';
+import { shallowEqual, useSelector } from 'react-redux';
+import { DashboardContextForExplore } from 'src/types/DashboardContextForExplore';
+import {
+  getItem,
+  LocalStorageKeys,
+  setItem,
+} from 'src/utils/localStorageHelpers';
+import { RootState } from 'src/dashboard/types';
+import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
+
+type Props = { dashboardPageId: string };
+
+const EMPTY_OBJECT = {};
+
+export const getDashboardContextLocalStorage = () => {
+  const dashboardsContexts = getItem(
+    LocalStorageKeys.dashboard__explore_context,
+    {},
+  );
+  // A new dashboard tab id is generated on each dashboard page opening.
+  // We mark ids as redundant when user leaves the dashboard, because they won't be reused.
+  // Then we remove redundant dashboard contexts from local storage in order not to clutter it
+  return Object.fromEntries(
+    Object.entries(dashboardsContexts).filter(
+      ([, value]) => !value.isRedundant,
+    ),
+  );
+};
+
+const updateDashboardTabLocalStorage = (
+  dashboardPageId: string,
+  dashboardContext: DashboardContextForExplore,
+) => {
+  const dashboardsContexts = getDashboardContextLocalStorage();
+  setItem(LocalStorageKeys.dashboard__explore_context, {
+    ...dashboardsContexts,
+    [dashboardPageId]: dashboardContext,
+  });
+};
+
+const SyncDashboardState: React.FC<Props> = ({ dashboardPageId }) => {
+  const dashboardContextForExplore = useSelector<
+    RootState,
+    DashboardContextForExplore
+  >(
+    ({ dashboardInfo, dashboardState, nativeFilters, dataMask }) => ({
+      labelColors: dashboardInfo.metadata?.label_colors || EMPTY_OBJECT,
+      sharedLabelColors:
+        dashboardInfo.metadata?.shared_label_colors || EMPTY_OBJECT,
+      colorScheme: dashboardState?.colorScheme,
+      chartConfiguration:
+        dashboardInfo.metadata?.chart_configuration || EMPTY_OBJECT,
+      nativeFilters: Object.entries(nativeFilters.filters).reduce(
+        (acc, [key, filterValue]) => ({
+          ...acc,
+          [key]: pick(filterValue, ['chartsInScope']),
+        }),
+        {},
+      ),
+      dataMask,
+      dashboardId: dashboardInfo.id,
+      filterBoxFilters: getActiveFilters(),
+      dashboardPageId,
+    }),
+    shallowEqual,
+  );
+
+  useEffect(() => {
+    updateDashboardTabLocalStorage(dashboardPageId, dashboardContextForExplore);
+    return () => {
+      // mark tab id as redundant when dashboard unmounts - case when user opens
+      // Explore in the same tab
+      updateDashboardTabLocalStorage(dashboardPageId, {
+        ...dashboardContextForExplore,
+        isRedundant: true,
+      });
+    };
+  }, [dashboardContextForExplore, dashboardPageId]);
+
+  return null;
+};
+
+export default SyncDashboardState;

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx

@@ -52,6 +52,7 @@ import {
   onFiltersRefreshSuccess,
   setDirectPathToChild,
 } from 'src/dashboard/actions/dashboardState';
+import { RESPONSIVE_WIDTH } from 'src/filters/components/common';
 import { FAST_DEBOUNCE } from 'src/constants';
 import { dispatchHoverAction, dispatchFocusAction } from './utils';
 import { FilterControlProps } from './types';
@@ -322,7 +323,7 @@ const FilterValue: React.FC<FilterControlProps> = ({
       ) : (
         <SuperChart
           height={HEIGHT}
-          width="100%"
+          width={RESPONSIVE_WIDTH}
           showOverflow={showOverflow}
           formData={formData}
           displaySettings={displaySettings}

superset-frontend/src/dashboard/containers/Dashboard.ts

@@ -39,7 +39,6 @@ function mapStateToProps(state: RootState) {
   const {
     datasources,
     sliceEntities,
-    charts,
     dataMask,
     dashboardInfo,
     dashboardState,
@@ -54,7 +53,6 @@ function mapStateToProps(state: RootState) {
     userId: dashboardInfo.userId,
     dashboardInfo,
     dashboardState,
-    charts,
     datasources,
     // filters prop: a map structure for all the active filter_box's values and scope in this dashboard,
     // for each filter field. map key is [chartId_column]

superset-frontend/src/dashboard/containers/DashboardPage.tsx

@@ -28,7 +28,6 @@ import {
   t,
   useTheme,
 } from '@superset-ui/core';
-import pick from 'lodash/pick';
 import { useDispatch, useSelector } from 'react-redux';
 import { useToasts } from 'src/components/MessageToasts/withToasts';
 import Loading from 'src/components/Loading';
@@ -42,11 +41,7 @@ import { setDatasources } from 'src/dashboard/actions/datasources';
 import injectCustomCss from 'src/dashboard/util/injectCustomCss';
 import setupPlugins from 'src/setup/setupPlugins';
 
-import {
-  getItem,
-  LocalStorageKeys,
-  setItem,
-} from 'src/utils/localStorageHelpers';
+import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers';
 import { URL_PARAMS } from 'src/constants';
 import { getUrlParam } from 'src/utils/urlUtils';
 import { getFilterSets } from 'src/dashboard/actions/nativeFilters';
@@ -55,25 +50,28 @@ import {
   getFilterValue,
   getPermalinkValue,
 } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue';
-import { DashboardContextForExplore } from 'src/types/DashboardContextForExplore';
+import DashboardContainer from 'src/dashboard/containers/Dashboard';
+
 import shortid from 'shortid';
 import { RootState } from '../types';
-import { getActiveFilters } from '../util/activeDashboardFilters';
 import {
   chartContextMenuStyles,
   filterCardPopoverStyle,
   headerStyles,
 } from '../styles';
+import SyncDashboardState, {
+  getDashboardContextLocalStorage,
+} from '../components/SyncDashboardState';
 
 export const DashboardPageIdContext = React.createContext('');
 
 setupPlugins();
-const DashboardContainer = React.lazy(
+const DashboardBuilder = React.lazy(
   () =>
     import(
       /* webpackChunkName: "DashboardContainer" */
       /* webpackPreload: true */
-      'src/dashboard/containers/Dashboard'
+      'src/dashboard/components/DashboardBuilder/DashboardBuilder'
     ),
 );
 
@@ -83,74 +81,15 @@ type PageProps = {
   idOrSlug: string;
 };
 
-const getDashboardContextLocalStorage = () => {
-  const dashboardsContexts = getItem(
-    LocalStorageKeys.dashboard__explore_context,
-    {},
-  );
-  // A new dashboard tab id is generated on each dashboard page opening.
-  // We mark ids as redundant when user leaves the dashboard, because they won't be reused.
-  // Then we remove redundant dashboard contexts from local storage in order not to clutter it
-  return Object.fromEntries(
-    Object.entries(dashboardsContexts).filter(
-      ([, value]) => !value.isRedundant,
-    ),
-  );
-};
-
-const updateDashboardTabLocalStorage = (
-  dashboardPageId: string,
-  dashboardContext: DashboardContextForExplore,
-) => {
-  const dashboardsContexts = getDashboardContextLocalStorage();
-  setItem(LocalStorageKeys.dashboard__explore_context, {
-    ...dashboardsContexts,
-    [dashboardPageId]: dashboardContext,
-  });
-};
-
-const useSyncDashboardStateWithLocalStorage = () => {
-  const dashboardPageId = useMemo(() => shortid.generate(), []);
-  const dashboardContextForExplore = useSelector<
-    RootState,
-    DashboardContextForExplore
-  >(({ dashboardInfo, dashboardState, nativeFilters, dataMask }) => ({
-    labelColors: dashboardInfo.metadata?.label_colors || {},
-    sharedLabelColors: dashboardInfo.metadata?.shared_label_colors || {},
-    colorScheme: dashboardState?.colorScheme,
-    chartConfiguration: dashboardInfo.metadata?.chart_configuration || {},
-    nativeFilters: Object.entries(nativeFilters.filters).reduce(
-      (acc, [key, filterValue]) => ({
-        ...acc,
-        [key]: pick(filterValue, ['chartsInScope']),
-      }),
-      {},
-    ),
-    dataMask,
-    dashboardId: dashboardInfo.id,
-    filterBoxFilters: getActiveFilters(),
-    dashboardPageId,
-  }));
-
-  useEffect(() => {
-    updateDashboardTabLocalStorage(dashboardPageId, dashboardContextForExplore);
-    return () => {
-      // mark tab id as redundant when dashboard unmounts - case when user opens
-      // Explore in the same tab
-      updateDashboardTabLocalStorage(dashboardPageId, {
-        ...dashboardContextForExplore,
-        isRedundant: true,
-      });
-    };
-  }, [dashboardContextForExplore, dashboardPageId]);
-  return dashboardPageId;
-};
-
 export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
   const theme = useTheme();
   const dispatch = useDispatch();
   const history = useHistory();
-  const dashboardPageId = useSyncDashboardStateWithLocalStorage();
+  const dashboardPageId = useMemo(() => shortid.generate(), []);
+  const hasDashboardInfoInitiated = useSelector<RootState, Boolean>(
+    ({ dashboardInfo }) =>
+      dashboardInfo && Object.keys(dashboardInfo).length > 0,
+  );
   const { addDangerToast } = useToasts();
   const { result: dashboard, error: dashboardApiError } =
     useDashboard(idOrSlug);
@@ -284,7 +223,7 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
   }, [addDangerToast, datasets, datasetsApiError, dispatch]);
 
   if (error) throw error; // caught in error boundary
-  if (!readyToRender || !isDashboardHydrated.current) return <Loading />;
+  if (!readyToRender || !hasDashboardInfoInitiated) return <Loading />;
 
   return (
     <>
@@ -295,8 +234,11 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
           chartContextMenuStyles(theme),
         ]}
       />
+      <SyncDashboardState dashboardPageId={dashboardPageId} />
       <DashboardPageIdContext.Provider value={dashboardPageId}>
-        <DashboardContainer />
+        <DashboardContainer>
+          <DashboardBuilder />
+        </DashboardContainer>
       </DashboardPageIdContext.Provider>
     </>
   );

superset-frontend/src/dataMask/reducer.ts

@@ -56,7 +56,6 @@ export function getInitialDataMask(
   }
   return {
     ...otherProps,
-    __cache: {},
     extraFormData: {},
     filterState: {},
     ownState: {},

superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx

@@ -91,15 +91,6 @@ describe('SelectFilterPlugin', () => {
   test('Add multiple values with first render', async () => {
     getWrapper();
     expect(setDataMask).toHaveBeenCalledWith({
-      extraFormData: {},
-      filterState: {
-        value: ['boy'],
-      },
-    });
-    expect(setDataMask).toHaveBeenCalledWith({
-      __cache: {
-        value: ['boy'],
-      },
       extraFormData: {
         filters: [
           {
@@ -118,9 +109,6 @@ describe('SelectFilterPlugin', () => {
     userEvent.click(screen.getByTitle('girl'));
     expect(await screen.findByTitle(/girl/i)).toBeInTheDocument();
     expect(setDataMask).toHaveBeenCalledWith({
-      __cache: {
-        value: ['boy'],
-      },
       extraFormData: {
         filters: [
           {
@@ -146,9 +134,6 @@ describe('SelectFilterPlugin', () => {
       }),
     );
     expect(setDataMask).toHaveBeenCalledWith({
-      __cache: {
-        value: ['boy'],
-      },
       extraFormData: {
         adhoc_filters: [
           {
@@ -174,9 +159,6 @@ describe('SelectFilterPlugin', () => {
       }),
     );
     expect(setDataMask).toHaveBeenCalledWith({
-      __cache: {
-        value: ['boy'],
-      },
       extraFormData: {},
       filterState: {
         label: undefined,
@@ -191,9 +173,6 @@ describe('SelectFilterPlugin', () => {
     expect(await screen.findByTitle('girl')).toBeInTheDocument();
     userEvent.click(screen.getByTitle('girl'));
     expect(setDataMask).toHaveBeenCalledWith({
-      __cache: {
-        value: ['boy'],
-      },
       extraFormData: {
         filters: [
           {
@@ -216,9 +195,6 @@ describe('SelectFilterPlugin', () => {
     expect(await screen.findByRole('combobox')).toBeInTheDocument();
     userEvent.click(screen.getByTitle(NULL_STRING));
     expect(setDataMask).toHaveBeenLastCalledWith({
-      __cache: {
-        value: ['boy'],
-      },
       extraFormData: {
         filters: [
           {

superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx

@@ -37,7 +37,6 @@ import { Select } from 'src/components';
 import { SLOW_DEBOUNCE } from 'src/constants';
 import { hasOption, propertyComparator } from 'src/components/Select/utils';
 import { FilterBarOrientation } from 'src/dashboard/types';
-import { uniqWith, isEqual } from 'lodash';
 import { PluginFilterSelectProps, SelectValue } from './types';
 import { FilterPluginStyle, StatusMessage, StyledFormItem } from '../common';
 import { getDataRecordFormatter, getSelectExtraFormData } from '../../utils';
@@ -46,15 +45,11 @@ type DataMaskAction =
   | { type: 'ownState'; ownState: JsonObject }
   | {
       type: 'filterState';
-      __cache: JsonObject;
       extraFormData: ExtraFormData;
       filterState: { value: SelectValue; label?: string };
     };
 
-function reducer(
-  draft: DataMask & { __cache?: JsonObject },
-  action: DataMaskAction,
-) {
+function reducer(draft: DataMask, action: DataMaskAction) {
   switch (action.type) {
     case 'ownState':
       draft.ownState = {
@@ -63,10 +58,18 @@ function reducer(
       };
       return draft;
     case 'filterState':
-      draft.extraFormData = action.extraFormData;
-      // eslint-disable-next-line no-underscore-dangle
-      draft.__cache = action.__cache;
-      draft.filterState = { ...draft.filterState, ...action.filterState };
+      if (
+        JSON.stringify(draft.extraFormData) !==
+        JSON.stringify(action.extraFormData)
+      ) {
+        draft.extraFormData = action.extraFormData;
+      }
+      if (
+        JSON.stringify(draft.filterState) !== JSON.stringify(action.filterState)
+      ) {
+        draft.filterState = { ...draft.filterState, ...action.filterState };
+      }
+
       return draft;
     default:
       return draft;
@@ -130,7 +133,6 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
       const suffix = inverseSelection && values?.length ? t(' (excluded)') : '';
       dispatchDataMask({
         type: 'filterState',
-        __cache: filterState,
         extraFormData: getSelectExtraFormData(
           col,
           values,
@@ -219,16 +221,13 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
   }, [filterState.validateMessage, filterState.validateStatus]);
 
   const uniqueOptions = useMemo(() => {
-    const allOptions = [...data];
-    return uniqWith(allOptions, isEqual).map(row => {
-      const [value] = groupby.map(col => row[col]);
-      return {
-        label: labelFormatter(value, datatype),
-        value,
-        isNewOption: false,
-      };
-    });
-  }, [data, datatype, groupby, labelFormatter]);
+    const allOptions = new Set([...data.map(el => el[col])]);
+    return [...allOptions].map((value: string) => ({
+      label: labelFormatter(value, datatype),
+      value,
+      isNewOption: false,
+    }));
+  }, [data, datatype, col, labelFormatter]);
 
   const options = useMemo(() => {
     if (search && !multiSelect && !hasOption(search, uniqueOptions, true)) {

superset-frontend/src/filters/components/common.ts

@@ -20,9 +20,11 @@ import { styled } from '@superset-ui/core';
 import { PluginFilterStylesProps } from './types';
 import FormItem from '../../components/Form/FormItem';
 
+export const RESPONSIVE_WIDTH = 0;
+
 export const FilterPluginStyle = styled.div<PluginFilterStylesProps>`
   min-height: ${({ height }) => height}px;
-  width: ${({ width }) => width}px;
+  width: ${({ width }) => (width === RESPONSIVE_WIDTH ? '100%' : `${width}px`)};
 `;
 
 export const StyledFormItem = styled(FormItem)`