Merge pull request #9866 from sahil143/odc-6263

Bug 1997108: fix react warnings while loading topology page

frontend/packages/dev-console/src/plugin.tsx

@@ -92,14 +92,7 @@ const plugin: Plugin<ConsumedExtensions> = [
     type: 'Page/Route',
     properties: {
       exact: true,
-      path: [
-        '/topology/all-namespaces',
-        '/topology/ns/:name',
-        '/topology/all-namespaces/graph',
-        '/topology/ns/:name/graph',
-        '/topology/all-namespaces/list',
-        '/topology/ns/:name/list',
-      ],
+      path: ['/topology/all-namespaces', '/topology/ns/:name'],
       loader: async () =>
         (
           await import(

frontend/packages/helm-plugin/src/utils/helm-utils.ts

@@ -175,7 +175,7 @@ export const getOriginRedirectURL = (
 ) => {
   switch (actionOrigin) {
     case HelmActionOrigins.topology:
-      return `/topology/ns/${namespace}/graph`;
+      return `/topology/ns/${namespace}?view=graph`;
     case HelmActionOrigins.list:
       return `/helm-releases/ns/${namespace}`;
     case HelmActionOrigins.details:

frontend/packages/topology/src/__tests__/TopologyPage.spec.tsx

@@ -2,7 +2,7 @@ import * as React from 'react';
 import { shallow } from 'enzyme';
 import NamespacedPage from '@console/dev-console/src/components/NamespacedPage';
 import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook';
-import { useUserSettingsCompatibility } from '@console/shared/src';
+import { useQueryParams, useUserSettingsCompatibility } from '@console/shared/src';
 import { TopologyPage } from '../components/page/TopologyPage';
 import { TopologyViewType } from '../topology-types';
 import { usePreferredTopologyView } from '../user-preferences/usePreferredTopologyView';
@@ -34,7 +34,7 @@ jest.mock('@console/shared', () => {
   const ActualShared = require.requireActual('@console/shared');
   return {
     ...ActualShared,
-    useQueryParams: () => new Map().set('view', mockViewParam),
+    useQueryParams: jest.fn(),
     useUserSettingsCompatibility: jest.fn(),
   };
 });
@@ -51,6 +51,7 @@ describe('Topology page tests', () => {
     (useK8sWatchResources as jest.Mock).mockReturnValue({
       projects: { data: [], loaded: true, loadError: '' },
     });
+    (useQueryParams as jest.Mock).mockReturnValue(new Map().set('view', mockViewParam));
   });
 
   it('should render topology page', () => {
@@ -79,10 +80,11 @@ describe('Topology page tests', () => {
   it('should render view from URL view path and ignore userSettings if it is available', () => {
     (useUserSettingsCompatibility as jest.Mock).mockReturnValue(['list', () => {}, true]);
     (usePreferredTopologyView as jest.Mock).mockReturnValue(['list', true]);
+    (useQueryParams as jest.Mock).mockReturnValue(new Map().set('view', 'graph'));
     const viewMatch = {
-      params: { name: 'default' },
+      params: { name: 'default', view: 'graph' },
       isExact: true,
-      path: '/topology/graph',
+      path: '/topology',
       url: '',
     };
     const wrapper = shallow(<TopologyPage match={viewMatch} hideProjects={false} />);
@@ -128,10 +130,11 @@ describe('Topology page tests', () => {
   it('should continue to support URL view path for graph', () => {
     (useUserSettingsCompatibility as jest.Mock).mockReturnValue(['', () => {}, true]);
     (useUserSettingsCompatibility as jest.Mock).mockReturnValue(['', () => {}]);
+    (useQueryParams as jest.Mock).mockReturnValue(new Map().set('view', 'graph'));
     const viewMatch = {
       params: { name: 'default' },
       isExact: true,
-      path: '/topology/graph',
+      path: '/topology',
       url: '',
     };
     const wrapper = shallow(<TopologyPage match={viewMatch} hideProjects={false} />);
@@ -141,10 +144,11 @@ describe('Topology page tests', () => {
   it('should continue to support URL view path for list', () => {
     (useUserSettingsCompatibility as jest.Mock).mockReturnValue(['', () => {}]);
     (usePreferredTopologyView as jest.Mock).mockReturnValue(['', true]);
+    (useQueryParams as jest.Mock).mockReturnValue(new Map().set('view', 'list'));
     const viewMatch = {
       params: { name: 'default' },
       isExact: true,
-      path: '/topology/list',
+      path: '/topology',
       url: '',
     };
     const wrapper = shallow(<TopologyPage match={viewMatch} hideProjects={false} />);

frontend/packages/topology/src/components/graph-view/Topology.tsx

@@ -146,7 +146,7 @@ const Topology: React.FC<TopologyProps &
     DynamicTopologyComponentFactory
   >(isDynamicTopologyComponentFactory);
 
-  const createVisualization = () => {
+  const createVisualization = React.useCallback(() => {
     const storedLayout = topologyLayoutDataJson?.[namespace];
     const newVisualization = new Visualization();
     newVisualization.registerElementFactory(odcElementFactory);
@@ -184,15 +184,19 @@ const Topology: React.FC<TopologyProps &
       const selectedEntity = ids[0] ? newVisualization.getElementById(ids[0]) : null;
       onSelect(selectedEntity);
     });
-    setVisualization(newVisualization);
     return newVisualization;
-  };
+  }, [namespace, onGraphModelChange, onSelect, setTopologyLayoutData, topologyLayoutDataJson]);
 
   const visualizationRef = React.useRef<Visualization>();
   if (!visualizationRef.current) {
     visualizationRef.current = createVisualization();
   }
   const visualization = visualizationRef.current;
+  React.useEffect(() => {
+    if (visualization) {
+      setVisualization(visualization);
+    }
+  }, [setVisualization, visualization]);
 
   React.useEffect(() => {
     if (model && visualizationReady) {

frontend/packages/topology/src/components/list-view/TopologyListView.tsx

@@ -153,7 +153,6 @@ const ConnectedTopologyListView: React.FC<TopologyListViewProps &
       const newVisualization = new Visualization();
       newVisualization.registerElementFactory(odcElementFactory);
       newVisualization.fromModel(listModel);
-      setVisualization(newVisualization);
       return newVisualization;
     };
 
@@ -164,6 +163,12 @@ const ConnectedTopologyListView: React.FC<TopologyListViewProps &
 
     const visualization = visualizationRef.current;
 
+    React.useEffect(() => {
+      if (visualization) {
+        setVisualization(visualization);
+      }
+    }, [setVisualization, visualization]);
+
     React.useEffect(() => {
       if (model) {
         // Clear out any layout that might have been saved

frontend/packages/topology/src/components/page/TopologyPage.tsx

@@ -2,7 +2,7 @@ import * as React from 'react';
 import { Button } from '@patternfly/react-core';
 import { Helmet } from 'react-helmet';
 import { useTranslation, Trans } from 'react-i18next';
-import { matchPath, match as RMatch } from 'react-router-dom';
+import { match as RMatch } from 'react-router-dom';
 import NamespacedPage, {
   NamespacedPageVariants,
 } from '@console/dev-console/src/components/NamespacedPage';
@@ -83,7 +83,7 @@ export const TopologyPage: React.FC<TopologyPageProps> = ({
 
   const loaded: boolean = preferredTopologyViewLoaded && isTopologyLastViewLoaded;
 
-  const getTopologyViewState = (): TopologyViewType => {
+  const topologyViewState = React.useMemo((): TopologyViewType => {
     if (!loaded) {
       return null;
     }
@@ -93,26 +93,18 @@ export const TopologyPage: React.FC<TopologyPageProps> = ({
     }
 
     return (preferredTopologyView || topologyLastView) as TopologyViewType;
-  };
+  }, [loaded, preferredTopologyView, topologyLastView]);
 
   const namespace = match.params.name;
   const queryParams = useQueryParams();
-  let viewType = queryParams.get('view') as TopologyViewType;
-  if (!viewType) {
-    // Backwards Compatibility, check path. Otherwise use any stored preference
-    viewType = matchPath(match.path, {
-      path: '*/list',
-      exact: true,
-    })
-      ? TopologyViewType.list
-      : matchPath(match.path, {
-          path: '*/graph',
-          exact: true,
-        })
-      ? TopologyViewType.graph
-      : loaded && (getTopologyViewState() || defaultViewType);
-    viewType && setQueryArgument('view', viewType);
-  }
+  const viewType =
+    (queryParams.get('view') as TopologyViewType) || topologyViewState || defaultViewType;
+
+  React.useEffect(() => {
+    if (!queryParams.get('view')) {
+      setQueryArgument('view', topologyViewState || defaultViewType);
+    }
+  }, [defaultViewType, topologyViewState, queryParams]);
 
   const onViewChange = React.useCallback(
     (newViewType: TopologyViewType) => {