perf: Prevent redundant calls to getRelevantDataMask (apache#30883)

preset-io · Nov 8, 2024 · 57af97d · 57af97d
1 parent f4c36a6
commit 57af97d
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 24 deletions.
diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx
@@ -62,6 +62,7 @@ const propTypes = {
   impressionId: PropTypes.string.isRequired,
   timeout: PropTypes.number,
   userId: PropTypes.string,
+  children: PropTypes.node,
 };
 
 const defaultProps = {

diff --git a/superset-frontend/src/dashboard/containers/Dashboard.ts b/superset-frontend/src/dashboard/containers/Dashboard.ts
@@ -28,23 +28,16 @@ import { setDatasources } from 'src/dashboard/actions/datasources';
 
 import { triggerQuery } from 'src/components/Chart/chartAction';
 import { logEvent } from 'src/logger/actions';
-import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
-import {
-  getAllActiveFilters,
-  getRelevantDataMask,
-} from 'src/dashboard/util/activeAllDashboardFilters';
 import { clearDataMaskState } from '../../dataMask/actions';
 
 function mapStateToProps(state: RootState) {
   const {
     datasources,
     sliceEntities,
-    dataMask,
     dashboardInfo,
     dashboardState,
     dashboardLayout,
     impressionId,
-    nativeFilters,
   } = state;
 
   return {
@@ -53,22 +46,7 @@ function mapStateToProps(state: RootState) {
     dashboardInfo,
     dashboardState,
     datasources,
-    // filters prop: a map structure for all the active filter's values and scope in this dashboard,
-    // for each filter field. map key is [chartId_column]
-    // When dashboard is first loaded into browser,
-    // its value is from preselect_filters that dashboard owner saved in dashboard's meta data
-    activeFilters: {
-      ...getActiveFilters(),
-      ...getAllActiveFilters({
-        // eslint-disable-next-line camelcase
-        chartConfiguration: dashboardInfo.metadata?.chart_configuration,
-        nativeFilters: nativeFilters.filters,
-        dataMask,
-        allSliceIds: dashboardState.sliceIds,
-      }),
-    },
     chartConfiguration: dashboardInfo.metadata?.chart_configuration,
-    ownDataCharts: getRelevantDataMask(dataMask, 'ownState'),
     slices: sliceEntities.slices,
     layout: dashboardLayout.present,
     impressionId,

diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx
@@ -21,6 +21,7 @@ import { Global } from '@emotion/react';
 import { useHistory } from 'react-router-dom';
 import { t, useTheme } from '@superset-ui/core';
 import { useDispatch, useSelector } from 'react-redux';
+import { createSelector } from '@reduxjs/toolkit';
 import { useToasts } from 'src/components/MessageToasts/withToasts';
 import Loading from 'src/components/Loading';
 import {
@@ -31,7 +32,11 @@ import {
 import { hydrateDashboard } from 'src/dashboard/actions/hydrate';
 import { setDatasources } from 'src/dashboard/actions/datasources';
 import injectCustomCss from 'src/dashboard/util/injectCustomCss';
-
+import {
+  getAllActiveFilters,
+  getRelevantDataMask,
+} from 'src/dashboard/util/activeAllDashboardFilters';
+import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
 import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers';
 import { URL_PARAMS } from 'src/constants';
 import { getUrlParam } from 'src/utils/urlUtils';
@@ -72,6 +77,33 @@ type PageProps = {
   idOrSlug: string;
 };
 
+// TODO: move to Dashboard.jsx when it's refactored to functional component
+const selectRelevantDatamask = createSelector(
+  (state: RootState) => state.dataMask, // the first argument accesses relevant data from global state
+  dataMask => getRelevantDataMask(dataMask, 'ownState'), // the second parameter conducts the transformation
+);
+
+// TODO: move to Dashboard.jsx when it's refactored to functional component
+const selectActiveFilters = createSelector(
+  (state: RootState) => ({
+    // eslint-disable-next-line camelcase
+    chartConfiguration: state.dashboardInfo.metadata?.chart_configuration,
+    nativeFilters: state.nativeFilters.filters,
+    dataMask: state.dataMask,
+    allSliceIds: state.dashboardState.sliceIds,
+  }),
+  ({ chartConfiguration, nativeFilters, dataMask, allSliceIds }) => ({
+    ...getActiveFilters(),
+    ...getAllActiveFilters({
+      // eslint-disable-next-line camelcase
+      chartConfiguration,
+      nativeFilters,
+      dataMask,
+      allSliceIds,
+    }),
+  }),
+);
+
 export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
   const theme = useTheme();
   const dispatch = useDispatch();
@@ -191,6 +223,9 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
     }
   }, [addDangerToast, datasets, datasetsApiError, dispatch]);
 
+  const relevantDataMask = useSelector(selectRelevantDatamask);
+  const activeFilters = useSelector(selectActiveFilters);
+
   if (error) throw error; // caught in error boundary
 
   return (
@@ -208,7 +243,10 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
         <>
           <SyncDashboardState dashboardPageId={dashboardPageId} />
           <DashboardPageIdContext.Provider value={dashboardPageId}>
-            <DashboardContainer>
+            <DashboardContainer
+              activeFilters={activeFilters}
+              ownDataCharts={relevantDataMask}
+            >
               <DashboardBuilder />
             </DashboardContainer>
           </DashboardPageIdContext.Provider>