From 57af97d1a2c61a9f3004e0de1f05961a8a3f369b Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 8 Nov 2024 19:09:20 +0100 Subject: [PATCH] perf: Prevent redundant calls to getRelevantDataMask (#30883) --- .../src/dashboard/components/Dashboard.jsx | 1 + .../src/dashboard/containers/Dashboard.ts | 22 ---------- .../dashboard/containers/DashboardPage.tsx | 42 ++++++++++++++++++- 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx index ddbc2441a5237..5bdce6d72ecfa 100644 --- 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 index b4808ecbd000f..ab1f7c46fa40a 100644 --- 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 index ab2256bcea075..d8ddf0f19e73a 100644 --- 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 = ({ idOrSlug }: PageProps) => { const theme = useTheme(); const dispatch = useDispatch(); @@ -191,6 +223,9 @@ export const DashboardPage: FC = ({ 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 = ({ idOrSlug }: PageProps) => { <> - +