Merge pull request #2935 from headlamp-k8s/sidebar-memo-fix

frontend: Fix incorrect links in sidebar after switching clusters

Author: joaquimrocha

frontend/src/components/App/TopBar.tsx

@@ -13,7 +13,7 @@ import { has } from 'lodash';
 import React, { memo, useCallback } from 'react';
 import { useTranslation } from 'react-i18next';
 import { useDispatch } from 'react-redux';
-import { useHistory } from 'react-router-dom';
+import { useHistory, useLocation } from 'react-router-dom';
 import helpers from '../../helpers';
 import { getToken, setToken } from '../../lib/auth';
 import { useCluster, useClustersConf } from '../../lib/k8s';
@@ -71,6 +71,10 @@ export default function TopBar({}: TopBarProps) {
   const history = useHistory();
   const { appBarActions, appBarActionsProcessors } = useAppBarActionsProcessed();
 
+  // getToken may return stale value so we need to rerender on navigation
+  // TODO: create a useToken hook that always returns latest token
+  // eslint-disable-next-line no-unused-vars
+  const _location = useLocation();
   function hasToken() {
     return !!cluster ? !!getToken(cluster) : false;
   }

frontend/src/components/Sidebar/SidebarItem.tsx

@@ -4,8 +4,9 @@ import ListItem, { ListItemProps } from '@mui/material/ListItem';
 import { useTheme } from '@mui/system';
 import React, { memo } from 'react';
 import { generatePath } from 'react-router';
+import { useCluster } from '../../lib/k8s';
 import { createRouteURL, getRoute } from '../../lib/router';
-import { getCluster, getClusterPrefixedPath } from '../../lib/util';
+import { getClusterPrefixedPath } from '../../lib/util';
 import ListItemLink from './ListItemLink';
 import { SidebarEntry } from './sidebarSlice';
 
@@ -45,12 +46,13 @@ const SidebarItem = memo((props: SidebarItemProps) => {
   } = props;
   const hasSubtitle = !!subtitle;
   const theme = useTheme();
+  const cluster = useCluster();
 
   // const classes = useItemStyle({ fullWidth, hasSubtitle: !!subtitle });
 
   let fullURL = url;
-  if (fullURL && useClusterURL && getCluster()) {
-    fullURL = generatePath(getClusterPrefixedPath(url), { cluster: getCluster()! });
+  if (fullURL && useClusterURL && cluster) {
+    fullURL = generatePath(getClusterPrefixedPath(url), { cluster });
   }
 
   if (!fullURL) {

frontend/src/lib/k8s/index.ts

@@ -1,6 +1,6 @@
 import _ from 'lodash';
 import React, { useMemo } from 'react';
-import { matchPath, useLocation } from 'react-router-dom';
+import { matchPath, useHistory } from 'react-router-dom';
 import { ConfigState } from '../../redux/configSlice';
 import { useTypedSelector } from '../../redux/reducers/reducers';
 import { getCluster, getClusterGroup, getClusterPrefixedPath } from '../util';
@@ -106,15 +106,13 @@ export function useClustersConf(): ConfigState['allClusters'] {
 }
 
 export function useCluster() {
-  // Make sure we update when changing clusters.
-  // @todo: We need a better way to do this.
-  const location = useLocation();
+  const history = useHistory();
 
   // This function is similar to the getCluster() but uses the location
   // meaning it will return the URL from whatever the router used it (which
   // is more accurate than getting it from window.location like the former).
   function getClusterFromLocation(): string | null {
-    const urlPath = location?.pathname;
+    const urlPath = history.location?.pathname;
     const clusterURLMatch = matchPath<{ cluster?: string }>(urlPath, {
       path: getClusterPrefixedPath(),
     });
@@ -124,11 +122,13 @@ export function useCluster() {
   const [cluster, setCluster] = React.useState<string | null>(getClusterFromLocation());
 
   React.useEffect(() => {
-    const currentCluster = getClusterFromLocation();
-    if (cluster !== currentCluster) {
-      setCluster(currentCluster);
-    }
-  }, [cluster, location]);
+    // Listen to route changes
+    return history.listen(() => {
+      const newCluster = getClusterFromLocation();
+      // Update the state only when the cluster changes
+      setCluster(currentCluster => (newCluster !== currentCluster ? newCluster : currentCluster));
+    });
+  }, [history]);
 
   return cluster;
 }