refactor: Centralize stable useSearchParams

coder · Parkreiner · Sep 20, 2023 · Sep 18, 2023 · Sep 18, 2023 · Sep 18, 2023
commit b06d77218b4bef58915fe49e9a198f83ba787d37
diff --git a/site/src/components/Filter/filter.tsx b/site/src/components/Filter/filter.tsx
@@ -26,7 +26,6 @@ import Divider from "@mui/material/Divider";
 import OpenInNewOutlined from "@mui/icons-material/OpenInNewOutlined";
 
 import { useDebouncedFunction } from "hooks/debounce";
-import { useEffectEvent } from "hooks/hookPolyfills";
 
 export type PresetFilter = {
   name: string;
@@ -48,17 +47,18 @@ export const useFilter = ({
   onUpdate,
   searchParamsResult,
 }: UseFilterConfig) => {
+  const [searchParams, setSearchParams] = searchParamsResult;
+
   // Fully expect the initialValue functions to have some impurity (e.g. reading
   // from localStorage during a render path). (Ab)using useState's lazy
   // initialization mode to guarantee impurities only exist on mount. Pattern
   // has added benefit of locking down initialValue and ignoring any accidental
   // value changes on re-renders
   const [readonlyInitialQueryState] = useState(initialValue);
 
-  // React Router doesn't give setSearchParams a stable memory reference; need
-  // extra logic to prevent on-mount effect from running too often
-  const [searchParams, setSearchParams] = searchParamsResult;
-  const syncSearchParamsOnMount = useEffectEvent(() => {
+  // Sync the params with the value provided via the initialValue function;
+  // should behave only as an on-mount effect
+  useEffect(() => {
     setSearchParams((current) => {
       const currentFilter = current.get(useFilterParamsKey);
       if (currentFilter !== readonlyInitialQueryState) {
@@ -67,11 +67,7 @@ export const useFilter = ({
 
       return current;
     });
-  });
-
-  useEffect(() => {
-    syncSearchParamsOnMount();
-  }, [syncSearchParamsOnMount]);
+  }, [setSearchParams, readonlyInitialQueryState]);
 
   const update = (newValues: string | FilterValues) => {
     const serialized =

diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx
@@ -21,13 +21,28 @@ import { MONOSPACE_FONT_FAMILY } from "theme/constants";
 import TextField from "@mui/material/TextField";
 import { displayError } from "components/GlobalSnackbar/utils";
 import { getErrorMessage } from "api/errors";
+import { useEffectEvent } from "hooks/hookPolyfills";
+
+function useSafeSearchParams() {
+  // Have to wrap setSearchParams because React Router doesn't make sure that
+  // the function's memory reference stays stable on each render, even though
+  // its logic never changes, and it even has function update support
+  const [searchParams, setSearchParams] = useSearchParams();
+  const stableSetSearchParams = useEffectEvent(setSearchParams);
+
+  // Need this to be a tuple type, but can't use "as const", because that would
+  // make the whole array readonly and cause type mismatches
+  return [searchParams, stableSetSearchParams] as ReturnType<
+    typeof useSearchParams
+  >;
+}
 
 const WorkspacesPage: FC = () => {
   const [dormantWorkspaces, setDormantWorkspaces] = useState<Workspace[]>([]);
   // If we use a useSearchParams for each hook, the values will not be in sync.
   // So we have to use a single one, centralizing the values, and pass it to
   // each hook.
-  const searchParamsResult = useSearchParams();
+  const searchParamsResult = useSafeSearchParams();
   const pagination = usePagination({ searchParamsResult });
   const filterProps = useWorkspacesFilter({ searchParamsResult, pagination });
   const { data, error, queryKey, refetch } = useWorkspacesData({