[Data Explorer] Allow render from View directly, not from Data Explor…

…er (#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 b792242)

Signed-off-by: Anan Zhuang <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

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<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} />;
 };

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 <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 { 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 (
-    <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 />
+    // 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>
-  );
-};
+                  <EuiResizablePanel initialSize={80} minSize="65%" mode="main" paddingSize="none">
+                    <EuiPageBody className="deLayout__canvas">
+                      <MemoizedCanvas {...params} />
+                    </EuiPageBody>
+                  </EuiResizablePanel>
+                </>
+              )}
+            </EuiResizableContainer>
+          </Context>
+        </Suspense>
+      </EuiPage>
+    );
+  },
+  (prevProps, nextProps) => {
+    return (
+      prevProps.view === nextProps.view &&
+      shallowEqual(prevProps.params, nextProps.params, ['history'])
+    );
+  }
+);

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);
+  });
+});

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;
+}

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<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;
@@ -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$,