From 0af3b500cfdbd7e5453495a23b770413151386d6 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Mon, 18 Mar 2024 05:44:49 +0000 Subject: [PATCH 1/2] [Discover] Remove double render This PR avoids re-rendering the entire AppContainer when data services update. The re-render is caused by history object which is in AppMountParameters. This history object is part of the application's routing context and can change frequently as the url is updated by query, filter or time range. Each change in the history object could potentially trigger a re-render of the AppContainer and its child components. With this PR, the AppContainer will not be re-loaded. Instead each component, like Canvas and Panel, will be updated. Signed-off-by: Anan Zhuang --- .../data_explorer/public/components/app.tsx | 24 +--- .../public/components/app_container.tsx | 114 +++++++++++------- .../view_components/utils/use_search.ts | 22 +++- 3 files changed, 92 insertions(+), 68 deletions(-) diff --git a/src/plugins/data_explorer/public/components/app.tsx b/src/plugins/data_explorer/public/components/app.tsx index ff6b5931a404..6b19413d17e3 100644 --- a/src/plugins/data_explorer/public/components/app.tsx +++ b/src/plugins/data_explorer/public/components/app.tsx @@ -3,34 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useEffect } from 'react'; -import { useLocation } from 'react-router-dom'; +import React from 'react'; import { AppMountParameters } from '../../../../core/public'; import { useView } from '../utils/use'; import { AppContainer } from './app_container'; -import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public'; -import { DataExplorerServices } from '../types'; -import { syncQueryStateWithUrl } from '../../../data/public'; export const DataExplorerApp = ({ params }: { params: AppMountParameters }) => { const { view } = useView(); - const { - services: { - data: { query }, - osdUrlStateStorage, - }, - } = useOpenSearchDashboards(); - const { pathname } = useLocation(); - - useEffect(() => { - // syncs `_g` portion of url with query services - const { stop } = syncQueryStateWithUrl(query, osdUrlStateStorage); - - return () => stop(); - - // this effect should re-run when pathname is changed to preserve querystring part, - // so the global state is always preserved - }, [query, osdUrlStateStorage, pathname]); - return ; }; diff --git a/src/plugins/data_explorer/public/components/app_container.tsx b/src/plugins/data_explorer/public/components/app_container.tsx index 529829140057..ef161d54e501 100644 --- a/src/plugins/data_explorer/public/components/app_container.tsx +++ b/src/plugins/data_explorer/public/components/app_container.tsx @@ -12,49 +12,75 @@ import { NoView } from './no_view'; import { View } from '../services/view_service/view'; import './app_container.scss'; -export const AppContainer = ({ view, params }: { view?: View; params: AppMountParameters }) => { - const isMobile = useIsWithinBreakpoints(['xs', 's', 'm']); - // TODO: Make this more robust. - if (!view) { - return ; +export const AppContainer = React.memo( + ({ view, params }: { view?: View; params: AppMountParameters }) => { + const isMobile = useIsWithinBreakpoints(['xs', 's', 'm']); + // TODO: Make this more robust. + if (!view) { + return ; + } + + const { Canvas, Panel, Context } = view; + + const MemoizedPanel = memo(Panel); + const MemoizedCanvas = memo(Canvas); + + // Render the application DOM. + return ( + + {/* TODO: improve fallback state */} + Loading...}> + + + {(EuiResizablePanel, EuiResizableButton) => ( + <> + + + + + + + + + + + + + + )} + + + + + ); + }, + (prevProps, nextProps) => { + return ( + prevProps.view === nextProps.view && + shallowEqual(prevProps.params, nextProps.params, ['history']) + ); + } +); + +// A simple shallow equal function that can ignore specified keys +function shallowEqual(object1: any, object2: any, ignoreKeys: any) { + const keys1 = Object.keys(object1).filter((key) => !ignoreKeys.includes(key)); + const keys2 = Object.keys(object2).filter((key) => !ignoreKeys.includes(key)); + + if (keys1.length !== keys2.length) { + return false; + } + + for (const key of keys1) { + if (object1[key] !== object2[key]) { + return false; + } } - const { Canvas, Panel, Context } = view; - - const MemoizedPanel = memo(Panel); - const MemoizedCanvas = memo(Canvas); - - // Render the application DOM. - return ( - - {/* TODO: improve fallback state */} - Loading...}> - - - {(EuiResizablePanel, EuiResizableButton) => ( - <> - - - - - - - - - - - - - - )} - - - - - ); -}; + return true; +} diff --git a/src/plugins/discover/public/application/view_components/utils/use_search.ts b/src/plugins/discover/public/application/view_components/utils/use_search.ts index 9da9704f32a7..e3a84ae84638 100644 --- a/src/plugins/discover/public/application/view_components/utils/use_search.ts +++ b/src/plugins/discover/public/application/view_components/utils/use_search.ts @@ -9,6 +9,7 @@ import { debounceTime } from 'rxjs/operators'; import { i18n } from '@osd/i18n'; import { useEffect } from 'react'; import { cloneDeep } from 'lodash'; +import { useLocation } from 'react-router-dom'; import { RequestAdapter } from '../../../../../inspector/public'; import { DiscoverViewServices } from '../../../build_services'; import { search } from '../../../../../data/public'; @@ -31,6 +32,7 @@ import { getResponseInspectorStats, } from '../../../opensearch_dashboards_services'; import { SEARCH_ON_PAGE_LOAD_SETTING } from '../../../../common'; +import { syncQueryStateWithUrl } from '../../../../../data/public'; export enum ResultStatus { UNINITIALIZED = 'uninitialized', @@ -68,11 +70,19 @@ export type RefetchSubject = Subject; * }, [data$]); */ export const useSearch = (services: DiscoverViewServices) => { + const { pathname } = useLocation(); const initalSearchComplete = useRef(false); const [savedSearch, setSavedSearch] = useState(undefined); const { savedSearch: savedSearchId, sort, interval } = useSelector((state) => state.discover); - const { data, filterManager, getSavedSearchById, core, toastNotifications, chrome } = services; const indexPattern = useIndexPattern(services); + const { + data, + filterManager, + getSavedSearchById, + core, + toastNotifications, + osdUrlStateStorage, + } = services; const timefilter = data.query.timefilter.timefilter; const fetchStateRef = useRef<{ abortController: AbortController | undefined; @@ -309,6 +319,16 @@ export const useSearch = (services: DiscoverViewServices) => { // eslint-disable-next-line react-hooks/exhaustive-deps }, [getSavedSearchById, savedSearchId]); + useEffect(() => { + // syncs `_g` portion of url with query services + const { stop } = syncQueryStateWithUrl(data.query, osdUrlStateStorage); + + return () => stop(); + + // this effect should re-run when pathname is changed to preserve querystring part, + // so the global state is always preserved + }, [data.query, osdUrlStateStorage, pathname]); + return { data$, refetch$, From 56a33c7e8bf1f6ded5d5a3372a38faf697fa09be Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Wed, 5 Jun 2024 17:30:31 +0000 Subject: [PATCH 2/2] fix PR comment Signed-off-by: Anan Zhuang --- .../public/components/app_container.tsx | 19 +------- .../public/utils/use/shallow_equal.test.ts | 48 +++++++++++++++++++ .../public/utils/use/shallow_equal.ts | 22 +++++++++ 3 files changed, 71 insertions(+), 18 deletions(-) create mode 100644 src/plugins/data_explorer/public/utils/use/shallow_equal.test.ts create mode 100644 src/plugins/data_explorer/public/utils/use/shallow_equal.ts diff --git a/src/plugins/data_explorer/public/components/app_container.tsx b/src/plugins/data_explorer/public/components/app_container.tsx index ef161d54e501..bf4a02bd223b 100644 --- a/src/plugins/data_explorer/public/components/app_container.tsx +++ b/src/plugins/data_explorer/public/components/app_container.tsx @@ -10,6 +10,7 @@ import { AppMountParameters } from '../../../../core/public'; import { Sidebar } from './sidebar'; import { NoView } from './no_view'; import { View } from '../services/view_service/view'; +import { shallowEqual } from '../utils/use/shallow_equal'; import './app_container.scss'; export const AppContainer = React.memo( @@ -66,21 +67,3 @@ export const AppContainer = React.memo( ); } ); - -// A simple shallow equal function that can ignore specified keys -function shallowEqual(object1: any, object2: any, ignoreKeys: any) { - const keys1 = Object.keys(object1).filter((key) => !ignoreKeys.includes(key)); - const keys2 = Object.keys(object2).filter((key) => !ignoreKeys.includes(key)); - - if (keys1.length !== keys2.length) { - return false; - } - - for (const key of keys1) { - if (object1[key] !== object2[key]) { - return false; - } - } - - return true; -} diff --git a/src/plugins/data_explorer/public/utils/use/shallow_equal.test.ts b/src/plugins/data_explorer/public/utils/use/shallow_equal.test.ts new file mode 100644 index 000000000000..d30bb2ffd3db --- /dev/null +++ b/src/plugins/data_explorer/public/utils/use/shallow_equal.test.ts @@ -0,0 +1,48 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { shallowEqual } from './shallow_equal'; + +describe('shallowEqual', () => { + it('should return true for equal objects without ignored keys', () => { + const obj1 = { a: 1, b: 2, c: 3 }; + const obj2 = { a: 1, b: 2, c: 3 }; + const ignoreKeys = []; + + expect(shallowEqual(obj1, obj2, ignoreKeys)).toBe(true); + }); + + it('should return false for objects with different values', () => { + const obj1 = { a: 1, b: 2, c: 3 }; + const obj2 = { a: 1, b: 2, c: 4 }; + const ignoreKeys = []; + + expect(shallowEqual(obj1, obj2, ignoreKeys)).toBe(false); + }); + + it('should return false for objects with different keys', () => { + const obj1 = { a: 1, b: 2 }; + const obj2 = { a: 1, b: 2, c: 3 }; + const ignoreKeys = []; + + expect(shallowEqual(obj1, obj2, ignoreKeys)).toBe(false); + }); + + it('should return true for objects with different values but ignored', () => { + const obj1 = { a: 1, b: 2, c: 3 }; + const obj2 = { a: 1, b: 2, c: 4 }; + const ignoreKeys = ['c']; + + expect(shallowEqual(obj1, obj2, ignoreKeys)).toBe(true); + }); + + it('should return true for objects with different keys but ignored', () => { + const obj1 = { a: 1, b: 2 }; + const obj2 = { a: 1, b: 2, c: 4 }; + const ignoreKeys = ['c']; + + expect(shallowEqual(obj1, obj2, ignoreKeys)).toBe(true); + }); +}); diff --git a/src/plugins/data_explorer/public/utils/use/shallow_equal.ts b/src/plugins/data_explorer/public/utils/use/shallow_equal.ts new file mode 100644 index 000000000000..734d5b5cf64c --- /dev/null +++ b/src/plugins/data_explorer/public/utils/use/shallow_equal.ts @@ -0,0 +1,22 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +// A simple shallow equal function that can ignore specified keys +export function shallowEqual(object1: any, object2: any, ignoreKeys: any) { + const keys1 = Object.keys(object1).filter((key) => !ignoreKeys.includes(key)); + const keys2 = Object.keys(object2).filter((key) => !ignoreKeys.includes(key)); + + if (keys1.length !== keys2.length) { + return false; + } + + for (const key of keys1) { + if (object1[key] !== object2[key]) { + return false; + } + } + + return true; +}