Skip to content

Commit

Permalink
[Discover] Remove double render
Browse files Browse the repository at this point in the history
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 <ananzh@amazon.com>
  • Loading branch information
ananzh committed Jun 4, 2024
1 parent c6820db commit 0af3b50
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 68 deletions.
24 changes: 1 addition & 23 deletions src/plugins/data_explorer/public/components/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<DataExplorerServices>();
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 <AppContainer view={view} params={params} />;
};
114 changes: 70 additions & 44 deletions src/plugins/data_explorer/public/components/app_container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 <NoView />;
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 <NoView />;
}

const { Canvas, Panel, Context } = view;

const MemoizedPanel = memo(Panel);
const MemoizedCanvas = memo(Canvas);

// Render the application DOM.
return (
<EuiPage className="deLayout" paddingSize="none">
{/* TODO: improve fallback state */}
<Suspense fallback={<div>Loading...</div>}>
<Context {...params}>
<EuiResizableContainer direction={isMobile ? 'vertical' : 'horizontal'}>
{(EuiResizablePanel, EuiResizableButton) => (
<>

Check warning on line 36 in src/plugins/data_explorer/public/components/app_container.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_explorer/public/components/app_container.tsx#L36

Added line #L36 was not covered by tests
<EuiResizablePanel
initialSize={20}
minSize="260px"
mode={['collapsible', { position: 'top' }]}
paddingSize="none"
>
<Sidebar>
<MemoizedPanel {...params} />
</Sidebar>
</EuiResizablePanel>
<EuiResizableButton />

<EuiResizablePanel initialSize={80} minSize="65%" mode="main" paddingSize="none">
<EuiPageBody className="deLayout__canvas">
<MemoizedCanvas {...params} />
</EuiPageBody>
</EuiResizablePanel>
</>
)}
</EuiResizableContainer>
</Context>
</Suspense>
</EuiPage>
);
},
(prevProps, nextProps) => {
return (

Check warning on line 63 in src/plugins/data_explorer/public/components/app_container.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_explorer/public/components/app_container.tsx#L63

Added line #L63 was not covered by tests
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));

Check warning on line 73 in src/plugins/data_explorer/public/components/app_container.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_explorer/public/components/app_container.tsx#L72-L73

Added lines #L72 - L73 were not covered by tests

if (keys1.length !== keys2.length) {
return false;

Check warning on line 76 in src/plugins/data_explorer/public/components/app_container.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_explorer/public/components/app_container.tsx#L76

Added line #L76 was not covered by tests
}

for (const key of keys1) {

Check warning on line 79 in src/plugins/data_explorer/public/components/app_container.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_explorer/public/components/app_container.tsx#L79

Added line #L79 was not covered by tests
if (object1[key] !== object2[key]) {
return false;

Check warning on line 81 in src/plugins/data_explorer/public/components/app_container.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_explorer/public/components/app_container.tsx#L81

Added line #L81 was not covered by tests
}
}

const { Canvas, Panel, Context } = view;

const MemoizedPanel = memo(Panel);
const MemoizedCanvas = memo(Canvas);

// Render the application DOM.
return (
<EuiPage className="deLayout" paddingSize="none">
{/* TODO: improve fallback state */}
<Suspense fallback={<div>Loading...</div>}>
<Context {...params}>
<EuiResizableContainer direction={isMobile ? 'vertical' : 'horizontal'}>
{(EuiResizablePanel, EuiResizableButton) => (
<>
<EuiResizablePanel
initialSize={20}
minSize="260px"
mode={['collapsible', { position: 'top' }]}
paddingSize="none"
>
<Sidebar>
<MemoizedPanel {...params} />
</Sidebar>
</EuiResizablePanel>
<EuiResizableButton />

<EuiResizablePanel initialSize={80} minSize="65%" mode="main" paddingSize="none">
<EuiPageBody className="deLayout__canvas">
<MemoizedCanvas {...params} />
</EuiPageBody>
</EuiResizablePanel>
</>
)}
</EuiResizableContainer>
</Context>
</Suspense>
</EuiPage>
);
};
return true;

Check warning on line 85 in src/plugins/data_explorer/public/components/app_container.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/data_explorer/public/components/app_container.tsx#L85

Added line #L85 was not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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',
Expand Down Expand Up @@ -68,11 +70,19 @@ export type RefetchSubject = Subject<SearchRefetch>;
* }, [data$]);
*/
export const useSearch = (services: DiscoverViewServices) => {
const { pathname } = useLocation();
const initalSearchComplete = useRef(false);
const [savedSearch, setSavedSearch] = useState<SavedSearch | undefined>(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;
Expand Down Expand Up @@ -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$,
Expand Down

0 comments on commit 0af3b50

Please sign in to comment.