You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by st...@apache.org on 2022/10/31 05:36:06 UTC

[superset] branch master updated: perf(native-filters): reduce the re-rendering of native filter modal (#21781)

This is an automated email from the ASF dual-hosted git repository.

stephenlyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 66f166be0f perf(native-filters): reduce the re-rendering of native filter modal (#21781)
66f166be0f is described below

commit 66f166be0f5485b9a51c80aef703b9d8d0fd93d5
Author: Stephen Liu <75...@qq.com>
AuthorDate: Mon Oct 31 13:35:54 2022 +0800

    perf(native-filters): reduce the re-rendering of native filter modal (#21781)
---
 .../FilterBar/FilterConfigurationLink/index.tsx    | 28 ++++++----
 .../nativeFilters/FilterBar/Header/index.tsx       |  6 +--
 .../FiltersConfigModal/FiltersConfigModal.test.tsx |  3 +-
 .../FiltersConfigModal/FiltersConfigModal.tsx      | 62 +++++++++++++---------
 .../FiltersConfigModal/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 bb3c151710..e99171c7e8 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<FCBProps> = ({
   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<FCBProps> = ({
         {...getFilterBarTestId('create-filter')}
         buttonStyle="link"
         buttonSize="xsmall"
-        onClick={() => setOpen(true)}
+        onClick={handleClick}
       >
         {children}
       </HeaderButton>
@@ -72,4 +80,4 @@ export const FilterConfigurationLink: React.FC<FCBProps> = ({
   );
 };
 
-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 45019f58cd..f57bc01b02 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<HeaderProps> = ({ toggleFiltersBar }) => {
   const theme = useTheme();
   const filters = useFilters();
-  const filterValues = Object.values(filters);
+  const filterValues = useMemo(() => Object.values(filters), [filters]);
   const canEdit = useSelector<RootState, boolean>(
     ({ dashboardInfo }) => dashboardInfo.dash_edit_perm,
   );
@@ -109,4 +109,4 @@ const Header: FC<HeaderProps> = ({ 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 80126fe6a9..58ebcea548 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 63bca5b3d1..fa7ef238e8 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<string, FilterRemoval> = {};
+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<string[]>([]);
+  const [newFilterIds, setNewFilterIds] = useState<string[]>(
+    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<string, FilterRemoval>
-  >({});
+  >(DEFAULT_REMOVED_FILTERS);
 
   const [saveAlertVisible, setSaveAlertVisible] = useState<boolean>(false);
 
@@ -143,13 +151,14 @@ export function FiltersConfigModal({
   const [currentFilterId, setCurrentFilterId] = useState(
     initialCurrentFilterId,
   );
-  const [erroredFilters, setErroredFilters] = useState<string[]>([]);
+  const [erroredFilters, setErroredFilters] = useState<string[]>(
+    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<NativeFiltersForm>({
-    filters: {},
-  });
+  const [formValues, setFormValues] =
+    useState<NativeFiltersForm>(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<string[]>(
-    getInitialFilterOrder(),
-  );
+  const [orderedFilters, setOrderedFilters] =
+    useState<string[]>(initialFilterOrder);
 
   // State for rendered filter to improve performance
   const [renderedFilters, setRenderedFilters] = useState<string[]>([
@@ -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({
     </StyledModalWrapper>
   );
 }
+
+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 7717d1915b..06a566937d 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,