From 4b9dd6795112b5c219a6d6676dd546cb0717e44a Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 6 Jun 2024 17:52:01 -0700 Subject: [PATCH] [Data Explorer] Allow render from View directly, not from Data Explorer (#6167) (#6959) * [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. --------- (cherry picked from commit b7922427566e2b4bfdc4143407583bee95af4891) Signed-off-by: Anan Zhuang Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../data_explorer/public/components/app.tsx | 24 +---- .../public/components/app_container.tsx | 93 ++++++++++--------- .../public/utils/use/shallow_equal.test.ts | 48 ++++++++++ .../public/utils/use/shallow_equal.ts | 22 +++++ .../view_components/utils/use_search.ts | 22 ++++- 5 files changed, 143 insertions(+), 66 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.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..bf4a02bd223b 100644 --- a/src/plugins/data_explorer/public/components/app_container.tsx +++ b/src/plugins/data_explorer/public/components/app_container.tsx @@ -10,51 +10,60 @@ 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 = ({ 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 { Canvas, Panel, Context } = view; - const MemoizedPanel = memo(Panel); - const MemoizedCanvas = memo(Canvas); + const MemoizedPanel = memo(Panel); + const MemoizedCanvas = memo(Canvas); - // Render the application DOM. - return ( - - {/* TODO: improve fallback state */} - Loading...}> - - - {(EuiResizablePanel, EuiResizableButton) => ( - <> - - - - - - + // 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']) + ); + } +); 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; +} 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$,