From c4a66cc16cf0c79ad3b041ad5456cb319132b7c5 Mon Sep 17 00:00:00 2001 From: stephenLYZ <750188453@qq.com> Date: Sun, 16 Oct 2022 02:04:53 +0800 Subject: [PATCH 1/3] perf(native-filters): improve native filter modal performance --- .../FilterConfigurePane.tsx | 56 ++---- .../FiltersConfigForm/FiltersConfigForm.tsx | 9 +- .../FiltersConfigForm/utils.ts | 8 +- .../FiltersConfigModal/FiltersConfigModal.tsx | 173 ++++++++++++------ 4 files changed, 148 insertions(+), 98 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx index dba7e6bb30250..6d1bf719a68d8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx @@ -22,7 +22,7 @@ import FilterTitlePane from './FilterTitlePane'; import { FilterRemoval } from './types'; interface Props { - children: (filterId: string) => React.ReactNode; + children: React.ReactNode; getFilterTitle: (filterId: string) => string; onChange: (activeKey: string) => void; onAdd: (type: NativeFilterType) => void; @@ -62,40 +62,24 @@ const FilterConfigurePane: React.FC = ({ currentFilterId, filters, removedFilters, -}) => { - const active = filters.filter(id => id === currentFilterId)[0]; - return ( - - - onAdd(type)} - onRearrange={onRearrange} - onRemove={(id: string) => onRemove(id)} - restoreFilter={restoreFilter} - /> - - - {filters.map(id => ( -
- {children(id)} -
- ))} -
-
- ); -}; +}) => ( + + + onAdd(type)} + onRearrange={onRearrange} + onRemove={(id: string) => onRemove(id)} + restoreFilter={restoreFilter} + /> + + {children} + +); export default FilterConfigurePane; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index 43901ddec2914..24f93ae38bd06 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -332,6 +332,7 @@ const FiltersConfigForm = ( setErroredFilters, validateDependencies, getDependencySuggestion, + isActive, }: FiltersConfigFormProps, ref: React.RefObject, ) => { @@ -346,7 +347,7 @@ const FiltersConfigForm = ( string, any > | null>(null); - const forceUpdate = useForceUpdate(); + const forceUpdate = useForceUpdate(isActive); const [datasetDetails, setDatasetDetails] = useState>(); const defaultFormFilter = useMemo(() => ({}), []); const filters = form.getFieldValue('filters'); @@ -1240,6 +1241,8 @@ const FiltersConfigForm = ( ); }; -export default forwardRef( - FiltersConfigForm, +export default React.memo( + forwardRef( + FiltersConfigForm, + ), ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts index 2a0b7fcad857b..3f0b69b19d252 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts @@ -38,9 +38,13 @@ export const FILTER_SUPPORTED_TYPES = { filter_range: [GenericDataType.NUMERIC], }; -export const useForceUpdate = () => { +export const useForceUpdate = (isActive: boolean) => { const [, updateState] = React.useState({}); - return React.useCallback(() => updateState({}), []); + return React.useCallback(() => { + if (isActive) { + updateState({}); + } + }, [isActive]); }; export const setNativeFilterFieldValues = ( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx index fd4f7d550fe0e..63bca5b3d166d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx @@ -153,12 +153,15 @@ export function FiltersConfigModal({ const unsavedFiltersIds = newFilterIds.filter(id => !removedFilters[id]); // brings back a filter that was previously removed ("Undo") - const restoreFilter = (id: string) => { - const removal = removedFilters[id]; - // gotta clear the removal timeout to prevent the filter from getting deleted - if (removal?.isPending) clearTimeout(removal.timerId); - setRemovedFilters(current => ({ ...current, [id]: null })); - }; + const restoreFilter = useCallback( + (id: string) => { + const removal = removedFilters[id]; + // gotta clear the removal timeout to prevent the filter from getting deleted + if (removal?.isPending) clearTimeout(removal.timerId); + setRemovedFilters(current => ({ ...current, [id]: null })); + }, + [removedFilters], + ); const getInitialFilterOrder = () => Object.keys(filterConfigMap); // State for tracking the re-ordering of filters @@ -166,6 +169,11 @@ export function FiltersConfigModal({ getInitialFilterOrder(), ); + // State for rendered filter to improve performance + const [renderedFilters, setRenderedFilters] = useState([ + initialCurrentFilterId, + ]); + const getActiveFilterPanelKey = (filterId: string) => [ `${filterId}-${FilterPanels.configuration.key}`, `${filterId}-${FilterPanels.settings.key}`, @@ -175,7 +183,7 @@ export function FiltersConfigModal({ string | string[] >(getActiveFilterPanelKey(initialCurrentFilterId)); - const onTabChange = (filterId: string) => { + const handleTabChange = (filterId: string) => { setCurrentFilterId(filterId); setActiveFilterPanelKey(getActiveFilterPanelKey(filterId)); }; @@ -229,6 +237,7 @@ export function FiltersConfigModal({ if (!isSaving) { setOrderedFilters(getInitialFilterOrder()); } + setRenderedFilters([initialCurrentFilterId]); form.resetFields(['filters']); form.setFieldsValue({ changed: false }); }; @@ -379,7 +388,7 @@ export function FiltersConfigModal({ handleConfirmCancel(); } }; - const onRearrange = (dragIndex: number, targetIndex: number) => { + const handleRearrange = (dragIndex: number, targetIndex: number) => { const newOrderedFilter = [...orderedFilters]; const removed = newOrderedFilter.splice(dragIndex, 1)[0]; newOrderedFilter.splice(targetIndex, 0, removed); @@ -405,7 +414,7 @@ export function FiltersConfigModal({ return dependencyMap; }, [filterConfigMap, form]); - const validateDependencies = () => { + const validateDependencies = useCallback(() => { const dependencyMap = buildDependencyMap(); filterIds .filter(id => !removedFilters[id]) @@ -418,26 +427,35 @@ export function FiltersConfigModal({ form.setFields([field]); }); handleErroredFilters(); - }; + }, [ + buildDependencyMap, + filterIds, + form, + handleErroredFilters, + removedFilters, + ]); - const getDependencySuggestion = (filterId: string) => { - const dependencyMap = buildDependencyMap(); - const possibleDependencies = orderedFilters.filter( - key => key !== filterId && canBeUsedAsDependency(key), - ); - const found = possibleDependencies.find(filter => { - const dependencies = dependencyMap.get(filterId) || []; - dependencies.push(filter); - if (hasCircularDependency(dependencyMap, filterId)) { - dependencies.pop(); - return false; - } - return true; - }); - return found || possibleDependencies[0]; - }; + const getDependencySuggestion = useCallback( + (filterId: string) => { + const dependencyMap = buildDependencyMap(); + const possibleDependencies = orderedFilters.filter( + key => key !== filterId && canBeUsedAsDependency(key), + ); + const found = possibleDependencies.find(filter => { + const dependencies = dependencyMap.get(filterId) || []; + dependencies.push(filter); + if (hasCircularDependency(dependencyMap, filterId)) { + dependencies.pop(); + return false; + } + return true; + }); + return found || possibleDependencies[0]; + }, + [buildDependencyMap, canBeUsedAsDependency, orderedFilters], + ); - const onValuesChange = useMemo( + const handleValuesChange = useMemo( () => debounce((changes: any, values: NativeFiltersForm) => { const didChangeFilterName = @@ -466,32 +484,73 @@ export function FiltersConfigModal({ ); }, [removedFilters]); - const getForm = (id: string) => { - const isDivider = id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX); - return isDivider ? ( - - ) : ( - setActiveFilterPanelKey(key)} - isActive={currentFilterId === id} - setErroredFilters={setErroredFilters} - validateDependencies={validateDependencies} - getDependencySuggestion={getDependencySuggestion} - /> - ); - }; + useEffect(() => { + if (!renderedFilters.includes(currentFilterId)) { + setRenderedFilters([...renderedFilters, currentFilterId]); + } + }, [currentFilterId]); + + const handleActiveFilterPanelChange = useCallback( + key => setActiveFilterPanelKey(key), + [setActiveFilterPanelKey], + ); + + const formList = useMemo( + () => + orderedFilters.map(id => { + if (!renderedFilters.includes(id)) return null; + const isDivider = id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX); + const isActive = currentFilterId === id; + return ( +
+ {isDivider ? ( + + ) : ( + + )} +
+ ); + }), + [ + renderedFilters, + orderedFilters, + currentFilterId, + filterConfigMap, + form, + removedFilters, + restoreFilter, + getAvailableFilters, + activeFilterPanelKey, + validateDependencies, + getDependencySuggestion, + handleActiveFilterPanelChange, + ], + ); return ( - {(id: string) => getForm(id)} + {formList} From 431bbbeb12e55a90bea0af53e5b50a341193a22a Mon Sep 17 00:00:00 2001 From: stephenLYZ <750188453@qq.com> Date: Sun, 16 Oct 2022 02:33:43 +0800 Subject: [PATCH 2/3] fix lint --- .../nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts index 3f0b69b19d252..77e27a42110cf 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts @@ -38,7 +38,7 @@ export const FILTER_SUPPORTED_TYPES = { filter_range: [GenericDataType.NUMERIC], }; -export const useForceUpdate = (isActive: boolean) => { +export const useForceUpdate = (isActive = true) => { const [, updateState] = React.useState({}); return React.useCallback(() => { if (isActive) { From 4a8d89e06377aa3d5162275342c4fdb3e124fd76 Mon Sep 17 00:00:00 2001 From: stephenLYZ <750188453@qq.com> Date: Sun, 16 Oct 2022 17:14:21 +0800 Subject: [PATCH 3/3] fix test --- .../FiltersConfigModal/FilterConfigPane.test.tsx | 8 -------- .../FiltersConfigModal/FilterConfigurePane.tsx | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx index 3742d536326fb..223d5a35d00d1 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx @@ -26,7 +26,6 @@ const scrollMock = jest.fn(); Element.prototype.scroll = scrollMock; const defaultProps = { - children: jest.fn(), getFilterTitle: (id: string) => id, onChange: jest.fn(), onAdd: jest.fn(), @@ -63,13 +62,6 @@ beforeEach(() => { scrollMock.mockClear(); }); -test('renders form', async () => { - await act(async () => { - defaultRender(); - }); - expect(defaultProps.children).toHaveBeenCalledTimes(3); -}); - test('drag and drop', async () => { defaultRender(); // Drag the state and country filter above the product filter diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx index 6d1bf719a68d8..d02a6ba2c3165 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx @@ -22,7 +22,7 @@ import FilterTitlePane from './FilterTitlePane'; import { FilterRemoval } from './types'; interface Props { - children: React.ReactNode; + children?: React.ReactNode; getFilterTitle: (filterId: string) => string; onChange: (activeKey: string) => void; onAdd: (type: NativeFilterType) => void;