From 66f166be0f5485b9a51c80aef703b9d8d0fd93d5 Mon Sep 17 00:00:00 2001 From: Stephen Liu <750188453@qq.com> Date: Mon, 31 Oct 2022 13:35:54 +0800 Subject: [PATCH] perf(native-filters): reduce the re-rendering of native filter modal (#21781) --- .../FilterConfigurationLink/index.tsx | 28 ++++++--- .../nativeFilters/FilterBar/Header/index.tsx | 6 +- .../FiltersConfigModal.test.tsx | 3 +- .../FiltersConfigModal/FiltersConfigModal.tsx | 62 ++++++++++++------- .../NativeFiltersModal.test.tsx | 2 +- 5 files changed, 61 insertions(+), 40 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx index bb3c1517104e..e99171c7e839 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx @@ -16,17 +16,18 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { useDispatch } from 'react-redux'; import { setFilterConfiguration } from 'src/dashboard/actions/nativeFilters'; import Button from 'src/components/Button'; import { FilterConfiguration, styled } from '@superset-ui/core'; -import { FiltersConfigModal } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal'; +import FiltersConfigModal from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal'; import { getFilterBarTestId } from '..'; export interface FCBProps { createNewOnOpen?: boolean; dashboardId?: number; + children?: React.ReactNode; } const HeaderButton = styled(Button)` @@ -41,14 +42,21 @@ export const FilterConfigurationLink: React.FC = ({ const dispatch = useDispatch(); const [isOpen, setOpen] = useState(false); - function close() { + const close = useCallback(() => { setOpen(false); - } + }, [setOpen]); - async function submit(filterConfig: FilterConfiguration) { - dispatch(await setFilterConfiguration(filterConfig)); - close(); - } + const submit = useCallback( + async (filterConfig: FilterConfiguration) => { + dispatch(await setFilterConfiguration(filterConfig)); + close(); + }, + [dispatch, close], + ); + + const handleClick = useCallback(() => { + setOpen(true); + }, [setOpen]); return ( <> @@ -57,7 +65,7 @@ export const FilterConfigurationLink: React.FC = ({ {...getFilterBarTestId('create-filter')} buttonStyle="link" buttonSize="xsmall" - onClick={() => setOpen(true)} + onClick={handleClick} > {children} @@ -72,4 +80,4 @@ export const FilterConfigurationLink: React.FC = ({ ); }; -export default FilterConfigurationLink; +export default React.memo(FilterConfigurationLink); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx index 45019f58cdfc..f57bc01b02a4 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx @@ -18,7 +18,7 @@ */ /* eslint-disable no-param-reassign */ import { css, styled, t, useTheme } from '@superset-ui/core'; -import React, { FC } from 'react'; +import React, { FC, useMemo } from 'react'; import Icons from 'src/components/Icons'; import Button from 'src/components/Button'; import { useSelector } from 'react-redux'; @@ -74,7 +74,7 @@ const AddFiltersButtonContainer = styled.div` const Header: FC = ({ toggleFiltersBar }) => { const theme = useTheme(); const filters = useFilters(); - const filterValues = Object.values(filters); + const filterValues = useMemo(() => Object.values(filters), [filters]); const canEdit = useSelector( ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, ); @@ -109,4 +109,4 @@ const Header: FC = ({ toggleFiltersBar }) => { ); }; -export default Header; +export default React.memo(Header); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index 80126fe6a925..58ebcea548bc 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -32,8 +32,7 @@ import { TimeFilterPlugin, TimeGrainFilterPlugin, } from 'src/filters/components'; -import { - FiltersConfigModal, +import FiltersConfigModal, { FiltersConfigModalProps, } from './FiltersConfigModal'; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx index 63bca5b3d166..fa7ef238e8f7 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx @@ -23,7 +23,7 @@ import React, { useState, useRef, } from 'react'; -import { uniq, isEqual, sortBy, debounce } from 'lodash'; +import { uniq, isEqual, sortBy, debounce, isEmpty } from 'lodash'; import { Filter, FilterConfiguration, @@ -91,6 +91,12 @@ export interface FiltersConfigModalProps { } export const ALLOW_DEPENDENCIES = ['filter_select']; +const DEFAULT_EMPTY_FILTERS: string[] = []; +const DEFAULT_REMOVED_FILTERS: Record = {}; +const DEFAULT_FORM_VALUES: NativeFiltersForm = { + filters: {}, +}; + /** * This is the modal to configure all the dashboard-native filters. * Manages modal-level state, such as what filters are in the list, @@ -99,7 +105,7 @@ export const ALLOW_DEPENDENCIES = ['filter_select']; * Calls the `save` callback with the new FilterConfiguration object * when the user saves the filters. */ -export function FiltersConfigModal({ +function FiltersConfigModal({ isOpen, initialFilterId, createNewOnOpen, @@ -116,14 +122,16 @@ export function FiltersConfigModal({ // new filter ids belong to filters have been added during // this configuration session, and only exist in the form state until we submit. - const [newFilterIds, setNewFilterIds] = useState([]); + const [newFilterIds, setNewFilterIds] = useState( + DEFAULT_EMPTY_FILTERS, + ); // store ids of filters that have been removed with the time they were removed // so that we can disappear them after a few secs. // filters are still kept in state until form is submitted. const [removedFilters, setRemovedFilters] = useState< Record - >({}); + >(DEFAULT_REMOVED_FILTERS); const [saveAlertVisible, setSaveAlertVisible] = useState(false); @@ -143,13 +151,14 @@ export function FiltersConfigModal({ const [currentFilterId, setCurrentFilterId] = useState( initialCurrentFilterId, ); - const [erroredFilters, setErroredFilters] = useState([]); + const [erroredFilters, setErroredFilters] = useState( + DEFAULT_EMPTY_FILTERS, + ); // the form values are managed by the antd form, but we copy them to here // so that we can display them (e.g. filter titles in the tab headers) - const [formValues, setFormValues] = useState({ - filters: {}, - }); + const [formValues, setFormValues] = + useState(DEFAULT_FORM_VALUES); const unsavedFiltersIds = newFilterIds.filter(id => !removedFilters[id]); // brings back a filter that was previously removed ("Undo") @@ -162,12 +171,14 @@ export function FiltersConfigModal({ }, [removedFilters], ); - const getInitialFilterOrder = () => Object.keys(filterConfigMap); + const initialFilterOrder = useMemo( + () => Object.keys(filterConfigMap), + [filterConfigMap], + ); // State for tracking the re-ordering of filters - const [orderedFilters, setOrderedFilters] = useState( - getInitialFilterOrder(), - ); + const [orderedFilters, setOrderedFilters] = + useState(initialFilterOrder); // State for rendered filter to improve performance const [renderedFilters, setRenderedFilters] = useState([ @@ -225,17 +236,17 @@ export function FiltersConfigModal({ // After this, it should be as if the modal was just opened fresh. // Called when the modal is closed. const resetForm = (isSaving = false) => { - setNewFilterIds([]); + setNewFilterIds(DEFAULT_EMPTY_FILTERS); setCurrentFilterId(initialCurrentFilterId); - setRemovedFilters({}); + setRemovedFilters(DEFAULT_REMOVED_FILTERS); setSaveAlertVisible(false); - setFormValues({ filters: {} }); - setErroredFilters([]); + setFormValues(DEFAULT_FORM_VALUES); + setErroredFilters(DEFAULT_EMPTY_FILTERS); if (filterIds.length > 0) { setActiveFilterPanelKey(getActiveFilterPanelKey(filterIds[0])); } if (!isSaving) { - setOrderedFilters(getInitialFilterOrder()); + setOrderedFilters(initialFilterOrder); } setRenderedFilters([initialCurrentFilterId]); form.resetFields(['filters']); @@ -330,7 +341,7 @@ export function FiltersConfigModal({ // no form validation issues found, resets errored filters if (!erroredFiltersIds.length && erroredFilters.length > 0) { - setErroredFilters([]); + setErroredFilters(DEFAULT_EMPTY_FILTERS); return; } // form validation issues found, sets errored filters @@ -373,10 +384,9 @@ export function FiltersConfigModal({ const handleCancel = () => { const changed = form.getFieldValue('changed'); - const initialOrder = getInitialFilterOrder(); const didChangeOrder = - orderedFilters.length !== initialOrder.length || - orderedFilters.some((val, index) => val !== initialOrder[index]); + orderedFilters.length !== initialFilterOrder.length || + orderedFilters.some((val, index) => val !== initialFilterOrder[index]); if ( unsavedFiltersIds.length > 0 || form.isFieldsTouched() || @@ -479,9 +489,11 @@ export function FiltersConfigModal({ ); useEffect(() => { - setErroredFilters(prevErroredFilters => - prevErroredFilters.filter(f => !removedFilters[f]), - ); + if (!isEmpty(removedFilters)) { + setErroredFilters(prevErroredFilters => + prevErroredFilters.filter(f => !removedFilters[f]), + ); + } }, [removedFilters]); useEffect(() => { @@ -601,3 +613,5 @@ export function FiltersConfigModal({ ); } + +export default React.memo(FiltersConfigModal); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx index 7717d1915b11..06a566937d0b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx @@ -28,7 +28,7 @@ import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint'; import { AntdDropdown } from 'src/components'; import { Menu } from 'src/components/Menu'; import Alert from 'src/components/Alert'; -import { FiltersConfigModal } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal'; +import FiltersConfigModal from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal'; Object.defineProperty(window, 'matchMedia', { writable: true,