Feedback: better sync URL params and component state

Author: brendan-kellam

packages/web/src/app/[domain]/search/components/filterPanel/index.tsx

@@ -1,15 +1,14 @@
 'use client';
 
+import { FileIcon } from "@/components/ui/fileIcon";
 import { Repository, SearchResultFile } from "@/lib/types";
 import { cn, getRepoCodeHostInfo } from "@/lib/utils";
-import { SetStateAction, useCallback, useEffect, useState } from "react";
+import { LaptopIcon } from "@radix-ui/react-icons";
+import Image from "next/image";
+import { useRouter, useSearchParams } from "next/navigation";
+import { useEffect, useMemo } from "react";
 import { Entry } from "./entry";
 import { Filter } from "./filter";
-import Image from "next/image";
-import { LaptopIcon } from "@radix-ui/react-icons";
-import { FileIcon } from "@/components/ui/fileIcon";
-import { useSearchParams } from "next/navigation";
-import { useRouter } from "next/navigation";
 
 interface FilePanelProps {
     matches: SearchResultFile[];
@@ -34,7 +33,7 @@ export const FilterPanel = ({
         return value ? new Set(value.split(',')) : new Set();
     };
 
-    const [repos, setRepos] = useState<Record<string, Entry>>(() => {
+    const repos = useMemo(() => {
         const selectedRepos = getSelectedFromQuery(REPOS_QUERY_PARAM);
         return aggregateMatches(
             "Repository",
@@ -60,10 +59,10 @@ export const FilterPanel = ({
                     Icon,
                 };
             }
-        );
-    });
+        )
+    }, [searchParams]);
 
-    const [languages, setLanguages] = useState<Record<string, Entry>>(() => {
+    const languages = useMemo(() => {
         const selectedLanguages = getSelectedFromQuery(LANGUAGES_QUERY_PARAM);
         return aggregateMatches(
             "Language",
@@ -82,20 +81,7 @@ export const FilterPanel = ({
                 } satisfies Entry;
             }
         );
-    });
-
-    const onEntryClicked = useCallback((
-        key: string,
-        setter: (value: SetStateAction<Record<string, Entry>>) => void,
-    ) => {
-        setter((values) => ({
-            ...values,
-            [key]: {
-                ...values[key],
-                isSelected: !values[key].isSelected,
-            },
-        }));
-    }, []);
+    }, [searchParams]);
 
     // Calls `onFilterChanged` with the filtered list of matches
     // whenever the filter state changes.
@@ -113,47 +99,53 @@ export const FilterPanel = ({
 
     }, [matches, repos, languages, onFilterChanged, searchParams, router]);
 
-    // Updates the query params when the filter state changes
-    useEffect(() => {
-        const selectedRepos = Object.keys(repos).filter((key) => repos[key].isSelected);
-        const selectedLanguages = Object.keys(languages).filter((key) => languages[key].isSelected);
-
-        const newParams = new URLSearchParams(searchParams.toString());
-
-        if (selectedRepos.length > 0) {
-            newParams.set(REPOS_QUERY_PARAM, selectedRepos.join(','));
-        } else {
-            newParams.delete(REPOS_QUERY_PARAM);
-        }
-
-        if (selectedLanguages.length > 0) {
-            newParams.set(LANGUAGES_QUERY_PARAM, selectedLanguages.join(','));
-        } else {
-            newParams.delete(LANGUAGES_QUERY_PARAM);
-        }
-
-        // Only push if params actually changed
-        if (newParams.toString() !== searchParams.toString()) {
-            router.replace(`?${newParams.toString()}`, { scroll: false });
-        }
-    }, [repos, languages, searchParams, router]);
-
-    const numRepos = Object.keys(repos).length > 100 ? '100+' : Object.keys(repos).length;
-    const numLanguages = Object.keys(languages).length > 100 ? '100+' : Object.keys(languages).length;
+    const numRepos = useMemo(() => Object.keys(repos).length > 100 ? '100+' : Object.keys(repos).length, [repos]);
+    const numLanguages = useMemo(() => Object.keys(languages).length > 100 ? '100+' : Object.keys(languages).length, [languages]);
+
     return (
         <div className="p-3 flex flex-col gap-3 h-full">
             <Filter
                 title="Filter By Repository"
                 searchPlaceholder={`Filter ${numRepos} repositories`}
                 entries={Object.values(repos)}
-                onEntryClicked={(key) => onEntryClicked(key, setRepos)}
+                onEntryClicked={(key) => {
+                    const newRepos = { ...repos };
+                    newRepos[key].isSelected = !newRepos[key].isSelected;
+                    const selectedRepos = Object.keys(newRepos).filter((key) => newRepos[key].isSelected);
+                    const newParams = new URLSearchParams(searchParams.toString());
+
+                    if (selectedRepos.length > 0) {
+                        newParams.set(REPOS_QUERY_PARAM, selectedRepos.join(','));
+                    } else {
+                        newParams.delete(REPOS_QUERY_PARAM);
+                    }
+
+                    if (newParams.toString() !== searchParams.toString()) {
+                        router.replace(`?${newParams.toString()}`, { scroll: false });
+                    }
+                }}
                 className="max-h-[50%]"
             />
             <Filter
                 title="Filter By Language"
                 searchPlaceholder={`Filter ${numLanguages} languages`}
                 entries={Object.values(languages)}
-                onEntryClicked={(key) => onEntryClicked(key, setLanguages)}
+                onEntryClicked={(key) => {
+                    const newLanguages = { ...languages };
+                    newLanguages[key].isSelected = !newLanguages[key].isSelected;
+                    const selectedLanguages = Object.keys(newLanguages).filter((key) => newLanguages[key].isSelected);
+                    const newParams = new URLSearchParams(searchParams.toString());
+
+                    if (selectedLanguages.length > 0) {
+                        newParams.set(LANGUAGES_QUERY_PARAM, selectedLanguages.join(','));
+                    } else {
+                        newParams.delete(LANGUAGES_QUERY_PARAM);
+                    }
+
+                    if (newParams.toString() !== searchParams.toString()) {
+                        router.replace(`?${newParams.toString()}`, { scroll: false });
+                    }
+                }}
                 className="overflow-auto"
             />
         </div>