Merge pull request #2600 from headlamp-k8s/map-query-state-fix

frontend: Rewrite useQueryParamsState logic and add unit tests

frontend/src/components/resourceMap/useQueryParamsState.test.tsx

@@ -0,0 +1,89 @@
+import { act, renderHook } from '@testing-library/react';
+import { createMemoryHistory } from 'history';
+import { Router } from 'react-router-dom';
+import { useQueryParamsState } from './useQueryParamsState';
+
+describe('useQueryParamsState', () => {
+  it('should initialize with the initial state if no query param is present', () => {
+    const history = createMemoryHistory();
+    const wrapper = ({ children }: { children: React.ReactNode }) => (
+      <Router history={history}>{children}</Router>
+    );
+
+    const { result } = renderHook(() => useQueryParamsState('test', 'initial'), { wrapper });
+
+    expect(result.current[0]).toBe('initial');
+    expect(history.length).toBe(1); // make sure it's replaced and not appended
+  });
+
+  it('should initialize with the query param value if present', () => {
+    const history = createMemoryHistory();
+    history.replace('?test=value');
+
+    const wrapper = ({ children }: { children: React.ReactNode }) => (
+      <Router history={history}>{children}</Router>
+    );
+
+    const { result } = renderHook(() => useQueryParamsState('test', 'initial'), { wrapper });
+
+    expect(result.current[0]).toBe('value');
+    expect(history.length).toBe(1);
+  });
+
+  it('should update the query param value', () => {
+    const history = createMemoryHistory();
+    const wrapper = ({ children }: { children: React.ReactNode }) => (
+      <Router history={history}>{children}</Router>
+    );
+
+    const { result } = renderHook(() => useQueryParamsState<string>('test', 'initial'), {
+      wrapper,
+    });
+
+    act(() => {
+      result.current[1]('new-value');
+    });
+
+    expect(history.location.search).toBe('?test=new-value');
+    expect(result.current[0]).toBe('new-value');
+    expect(history.length).toBe(2);
+  });
+
+  it('should remove the query param if the new value is undefined', () => {
+    const history = createMemoryHistory();
+    history.replace('?test=value');
+
+    const wrapper = ({ children }: { children: React.ReactNode }) => (
+      <Router history={history}>{children}</Router>
+    );
+
+    const { result } = renderHook(() => useQueryParamsState('test', 'initial'), { wrapper });
+
+    act(() => {
+      result.current[1](undefined);
+    });
+
+    expect(history.location.search).toBe('');
+    expect(result.current[0]).toBeUndefined();
+    expect(history.length).toBe(2);
+  });
+
+  it('should replace the query param value if replace option is true', () => {
+    const history = createMemoryHistory();
+    const wrapper = ({ children }: { children: React.ReactNode }) => (
+      <Router history={history}>{children}</Router>
+    );
+
+    const { result } = renderHook(() => useQueryParamsState<string>('test', 'initial'), {
+      wrapper,
+    });
+
+    act(() => {
+      result.current[1]('new-value', { replace: true });
+    });
+
+    expect(history.location.search).toBe('?test=new-value');
+    expect(result.current[0]).toBe('new-value');
+    expect(history.length).toBe(1);
+  });
+});

frontend/src/components/resourceMap/useQueryParamsState.tsx

@@ -1,7 +1,10 @@
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback, useEffect, useMemo } from 'react';
 import { useHistory, useLocation } from 'react-router';
 
-type UseQueryParamsStateReturnType<T> = [T | undefined, (newValue: T | undefined) => void];
+type UseQueryParamsStateReturnType<T> = [
+  T | undefined,
+  (newValue: T | undefined, params?: { replace?: boolean }) => void
+];
 
 /**
  * Custom hook to manage a state synchronized with a URL query parameter
@@ -18,64 +21,45 @@ export function useQueryParamsState<T extends string | undefined>(
   param: string,
   initialState: T
 ): UseQueryParamsStateReturnType<T> {
-  const location = useLocation();
+  const { search } = useLocation();
   const history = useHistory();
 
-  // State for managing the value derived from the query parameter
-  const [value, setValue] = useState<T | undefined>(() => {
-    const { search } = location;
-    const searchParams = new URLSearchParams(search);
-    const paramValue = searchParams.get(param);
+  const value = useMemo(() => {
+    const params = new URLSearchParams(search);
+    return (params.get(param) ?? undefined) as T | undefined;
+  }, [search, param]);
 
-    return paramValue !== null ? (decodeURIComponent(paramValue) as T) : undefined;
-  });
-
-  // Update the value from URL to state
-  useEffect(() => {
-    const searchParams = new URLSearchParams(location.search);
-    const paramValue = searchParams.get(param);
-
-    if (paramValue !== null) {
-      const decodedValue = decodeURIComponent(paramValue) as T;
-      setValue(decodedValue);
-    } else {
-      setValue(undefined);
-    }
-  }, [location.search]);
-
-  // Set the value from state to URL
-  useEffect(() => {
-    const currentSearchParams = new URLSearchParams(location.search);
-
-    if (value && currentSearchParams.get(param) === encodeURIComponent(value)) return;
-
-    // Update the query parameter with the current state value
-    if (value !== null && value !== '' && value !== undefined) {
-      currentSearchParams.set(param, encodeURIComponent(value));
-    } else {
-      currentSearchParams.delete(param);
-    }
-
-    // Update the URL with the modified search parameters
-    const newUrl = [location.pathname, currentSearchParams.toString()].filter(Boolean).join('?');
-
-    history.push(newUrl);
-  }, [param, value]);
-
-  // Initi state with initial state value
-  useEffect(() => {
-    setValue(initialState);
-  }, []);
-
-  const handleSetValue = useCallback(
-    (newValue: T | undefined) => {
+  const setValue = useCallback(
+    (newValue: T | undefined, params: { replace?: boolean } = {}) => {
       if (newValue !== undefined && typeof newValue !== 'string') {
         throw new Error("useQueryParamsState: Can't set a value to something that isn't a string");
       }
-      setValue(newValue);
+
+      // Create new search params
+      const newParams = new URLSearchParams(history.location.search);
+      if (newValue === undefined) {
+        newParams.delete(param);
+      } else {
+        newParams.set(param, newValue);
+      }
+
+      // Apply new search params
+      const newSearch = '?' + newParams;
+      if (params.replace) {
+        history.replace(newSearch);
+      } else {
+        history.push(newSearch);
+      }
     },
-    [setValue]
+    [history.location.search, param]
   );
 
-  return [value, handleSetValue];
+  // Apply initialState if any
+  useEffect(() => {
+    if (initialState && !value) {
+      setValue(initialState, { replace: true });
+    }
+  }, [initialState]);
+
+  return [value, setValue];
 }