Remove view defaults logic from useQueryState hook

Author: yomete

ui/packages/shared/components/src/hooks/URLState/index.test.tsx

@@ -647,121 +647,4 @@ describe('URLState Hooks', () => {
       consoleSpy.mockRestore();
     });
   });
-
-  describe('mergeStrategy option', () => {
-    it('should replace existing value by default', async () => {
-      const {result} = renderHook(() => useURLState('param'), {wrapper: createWrapper()});
-      const [, setParam] = result.current;
-
-      act(() => {
-        setParam('initial');
-      });
-      await waitFor(() => {
-        expect(result.current[0]).toBe('initial');
-      });
-
-      act(() => {
-        setParam('replaced');
-      });
-      await waitFor(() => {
-        expect(result.current[0]).toBe('replaced');
-      });
-    });
-
-    it('should only set value when empty with preserve-existing strategy', async () => {
-      const {result} = renderHook(
-        () => useURLState('param', {mergeStrategy: 'preserve-existing'}),
-        {wrapper: createWrapper()}
-      );
-      const [, setParam] = result.current;
-
-      // Set when undefined - should work
-      act(() => {
-        setParam('first');
-      });
-      await waitFor(() => {
-        expect(result.current[0]).toBe('first');
-      });
-
-      // Try to overwrite - should be ignored
-      act(() => {
-        setParam('second');
-      });
-      await waitFor(() => {
-        expect(result.current[0]).toBe('first');
-      });
-    });
-
-    it('should merge arrays with deduplication using append strategy', async () => {
-      const {result} = renderHook(
-        () => useURLState<string | string[]>('param', {mergeStrategy: 'append'}),
-        {wrapper: createWrapper()}
-      );
-      const [, setParam] = result.current;
-
-      // Set initial array
-      act(() => {
-        setParam(['a', 'b']);
-      });
-      await waitFor(() => {
-        expect(result.current[0]).toEqual(['a', 'b']);
-      });
-
-      // Append with overlap - should deduplicate
-      act(() => {
-        setParam(['b', 'c']);
-      });
-      await waitFor(() => {
-        expect(result.current[0]).toEqual(['a', 'b', 'c']);
-      });
-
-      // Append string to array
-      act(() => {
-        setParam('d');
-      });
-      await waitFor(() => {
-        expect(result.current[0]).toEqual(['a', 'b', 'c', 'd']);
-      });
-
-      // Append duplicate string - should be ignored
-      act(() => {
-        setParam('a');
-      });
-      await waitFor(() => {
-        expect(result.current[0]).toEqual(['a', 'b', 'c', 'd']);
-      });
-    });
-
-    it('should return undefined and no-op setter when enabled is false', async () => {
-      const {result} = renderHook(() => useURLState('param', {enabled: false}), {
-        wrapper: createWrapper(),
-      });
-
-      const [value, setParam] = result.current;
-      expect(value).toBeUndefined();
-
-      act(() => {
-        setParam('should-not-work');
-      });
-      await waitFor(() => {
-        expect(mockNavigateTo).not.toHaveBeenCalled();
-      });
-    });
-
-    it('should handle compare mode with enabled option', async () => {
-      const TestComponent = (): {
-        groupByA: string | string[] | undefined;
-        groupByB: string | string[] | undefined;
-      } => {
-        const [groupByA] = useURLState('group_by', {enabled: true, defaultValue: ['node']});
-        const [groupByB] = useURLState('group_by', {enabled: false});
-        return {groupByA, groupByB};
-      };
-
-      const {result} = renderHook(() => TestComponent(), {wrapper: createWrapper()});
-
-      expect(result.current.groupByA).toEqual(['node']);
-      expect(result.current.groupByB).toBeUndefined();
-    });
-  });
 });

ui/packages/shared/components/src/hooks/URLState/index.tsx

@@ -210,8 +210,6 @@ interface Options {
   defaultValue?: string | string[];
   debugLog?: boolean;
   alwaysReturnArray?: boolean;
-  mergeStrategy?: 'replace' | 'append' | 'preserve-existing';
-  enabled?: boolean;
 }
 
 export const useURLState = <T extends ParamValue>(
@@ -223,13 +221,10 @@ export const useURLState = <T extends ParamValue>(
     throw new Error('useURLState must be used within a URLStateProvider');
   }
 
-  const {debugLog, defaultValue, alwaysReturnArray, mergeStrategy, enabled} = _options ?? {};
+  const {debugLog, defaultValue, alwaysReturnArray} = _options ?? {};
 
   const {state, setState} = context;
 
-  // Create no-op setter unconditionally to satisfy hooks rules
-  const noOpSetter = useCallback(() => {}, []);
-
   const setParam: ParamValueSetter = useCallback(
     (val: ParamValue) => {
       if (debugLog === true) {
@@ -238,57 +233,13 @@ export const useURLState = <T extends ParamValue>(
 
       // Just update state - Provider handles URL sync automatically!
       setState(currentState => {
-        const currentValue = currentState[param];
-        let newValue: ParamValue;
-
-        if (mergeStrategy === undefined || mergeStrategy === 'replace') {
-          newValue = val;
-        } else if (mergeStrategy === 'preserve-existing') {
-          // Only set if current is empty (including empty string)
-          if (
-            currentValue === undefined ||
-            currentValue === null ||
-            currentValue === '' ||
-            (Array.isArray(currentValue) && currentValue.length === 0)
-          ) {
-            newValue = val;
-          } else {
-            newValue = currentValue; // Keep existing
-          }
-        } else if (mergeStrategy === 'append') {
-          // Ignore undefined/null new values - keep current state
-          if (val === undefined || val === null) {
-            newValue = currentValue;
-          } else if (currentValue === undefined || currentValue === null || currentValue === '') {
-            // Current is empty, use new value
-            newValue = val;
-          } else if (Array.isArray(currentValue) && Array.isArray(val)) {
-            // Merge arrays and deduplicate
-            newValue = Array.from(new Set([...currentValue, ...val]));
-          } else if (Array.isArray(currentValue) && typeof val === 'string') {
-            // Add string to array if not present (deduplication)
-            newValue = currentValue.includes(val) ? currentValue : [...currentValue, val];
-          } else if (typeof currentValue === 'string' && Array.isArray(val)) {
-            // Merge string with array and deduplicate
-            newValue = Array.from(new Set([currentValue, ...val]));
-          } else if (typeof currentValue === 'string' && typeof val === 'string') {
-            // Create array from both strings (deduplicate)
-            newValue = currentValue === val ? currentValue : [currentValue, val];
-          } else {
-            // Fallback to replace for other cases
-            newValue = val;
-          }
-        } else {
-          newValue = val;
-        }
-
         return {
           ...currentState,
-          [param]: newValue,
+          [param]: val,
         };
       });
     },
-    [param, setState, debugLog, mergeStrategy]
+    [param, setState, debugLog]
   );
 
   if (debugLog === true) {
@@ -344,11 +295,6 @@ export const useURLState = <T extends ParamValue>(
     }
   }
 
-  // Return early if hook is disabled (after all hooks have been called)
-  if (enabled === false) {
-    return [undefined as T, noOpSetter];
-  }
-
   return [(value ?? defaultValue) as T, setParam];
 };
 

ui/packages/shared/profile/src/ProfileSelector/index.tsx

@@ -124,9 +124,7 @@ const ProfileSelector = ({
   const batchUpdates = useURLStateBatch();
 
   const profileFilterDefaults = viewComponent?.profileFilterDefaults as ProfileFilter[] | undefined;
-  const {forceApplyFilters} = useProfileFilters({
-    viewDefaults: profileFilterDefaults,
-  });
+  const {forceApplyFilters} = useProfileFilters();
 
   const handleProfileTypeChange = useCallback(() => {
     if (profileFilterDefaults != null && profileFilterDefaults.length > 0) {

ui/packages/shared/profile/src/ProfileView/components/ProfileFilters/useProfileFilters.ts

@@ -225,13 +225,7 @@ export const convertToProtoFilters = (profileFilters: ProfileFilter[]): Filter[]
     });
 };
 
-interface UseProfileFiltersOptions {
-  viewDefaults?: ProfileFilter[];
-}
-
-export const useProfileFilters = (
-  options: UseProfileFiltersOptions = {}
-): {
+export const useProfileFilters = (): {
   localFilters: ProfileFilter[];
   appliedFilters: ProfileFilter[];
   protoFilters: Filter[];
@@ -243,14 +237,10 @@ export const useProfileFilters = (
   removeFilter: (id: string) => void;
   updateFilter: (id: string, updates: Partial<ProfileFilter>) => void;
   resetFilters: () => void;
-  applyViewDefaults: () => void;
+  setAppliedFilters: (filters: ProfileFilter[]) => void;
   forceApplyFilters: (filters: ProfileFilter[]) => void;
 } => {
-  const {viewDefaults} = options;
-  const {appliedFilters, setAppliedFilters, applyViewDefaults, forceApplyFilters} =
-    useProfileFiltersUrlState({
-      viewDefaults,
-    });
+  const {appliedFilters, setAppliedFilters, forceApplyFilters} = useProfileFiltersUrlState();
   const resetFlameGraphState = useResetFlameGraphState();
 
   const [localFilters, setLocalFilters] = useState<ProfileFilter[]>(appliedFilters ?? []);
@@ -434,7 +424,7 @@ export const useProfileFilters = (
     removeFilter,
     updateFilter,
     resetFilters,
-    applyViewDefaults,
+    setAppliedFilters,
     forceApplyFilters,
   };
 };