wip: commit progress for searchbar fix

Parkreiner · Parkreiner · commit 54a5940e51c9 · 2023-09-19T18:05:52.000Z
diff --git a/site/src/components/Filter/filter.tsx b/site/src/components/Filter/filter.tsx
@@ -36,53 +36,23 @@ type FilterValues = Record<string, string | undefined>;
 
 type UseFilterConfig = {
   /**
-   * If initialValue is a string, that value will be used immediately from the
-   * first render.
-   *
-   * If it's a function, the function will be lazy-evaluated via an effect
-   * after the initial render. The initial render will use an empty string while
-   * waiting for the effect to run.
+   * The fallback value to use in the event that no filter params can be used.
+   * This value is allowed to change on re-renders.
    */
-  initialValue?: string | (() => string);
+  initialValue?: string;
   searchParamsResult: ReturnType<typeof useSearchParams>;
   onUpdate?: (newValue: string) => void;
 };
 
 const useFilterParamsKey = "filter";
 
 export const useFilter = ({
-  initialValue = "",
-  onUpdate,
+  initialValue: fallbackFilter = "",
   searchParamsResult,
+  onUpdate,
 }: UseFilterConfig) => {
   const [searchParams, setSearchParams] = searchParamsResult;
 
-  // Copying initialValue into state to lock the value down from the outside
-  const [initializedValue, setInitializedValue] = useState(() => {
-    return typeof initialValue === "string" ? initialValue : "";
-  });
-
-  // Lazy-evaluate initialValue only on mount; have to resolve via effect
-  // because initialValue is allowed to be impure (e.g., read from localStorage)
-  const lazyOnMountRef = useRef(initialValue);
-  useEffect(() => {
-    if (typeof lazyOnMountRef.current === "string") {
-      return;
-    }
-
-    const lazyInitialValue = lazyOnMountRef.current();
-    setInitializedValue(lazyInitialValue);
-
-    setSearchParams((current) => {
-      const currentFilter = current.get(useFilterParamsKey);
-      if (currentFilter !== lazyInitialValue) {
-        current.set(useFilterParamsKey, lazyInitialValue);
-      }
-
-      return current;
-    });
-  }, [setSearchParams]);
-
   const update = (newValues: string | FilterValues) => {
     const serialized =
       typeof newValues === "string" ? newValues : stringifyFilter(newValues);
@@ -100,9 +70,9 @@ export const useFilter = ({
     500,
   );
 
-  const query = searchParams.get("filter") ?? initializedValue;
+  const query = searchParams.get(useFilterParamsKey) ?? fallbackFilter;
   const values = parseFilterQuery(query);
-  const used = query !== "" && query !== initialValue;
+  const used = query !== "" && query !== fallbackFilter;
 
   return {
     query,
diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx
@@ -4,7 +4,7 @@ import {
   useDashboard,
   useIsWorkspaceActionsEnabled,
 } from "components/Dashboard/DashboardProvider";
-import { FC, useEffect, useState } from "react";
+import { type FC, useEffect, useState, useSyncExternalStore } from "react";
 import { Helmet } from "react-helmet-async";
 import { pageTitle } from "utils/page";
 import { useWorkspacesData, useWorkspaceUpdate } from "./data";
@@ -44,7 +44,11 @@ const WorkspacesPage: FC = () => {
   // each hook.
   const searchParamsResult = useSafeSearchParams();
   const pagination = usePagination({ searchParamsResult });
-  const filterProps = useWorkspacesFilter({ searchParamsResult, pagination });
+  const filterProps = useWorkspacesFilter({
+    searchParamsResult,
+    onPageChange: () => pagination.goToPage(1),
+  });
+
   const { data, error, queryKey, refetch } = useWorkspacesData({
     ...pagination,
     query: filterProps.filter.query,
@@ -136,30 +140,61 @@ const WorkspacesPage: FC = () => {
 
 export default WorkspacesPage;
 
+const workspaceFilterKey = "WorkspacesPage/filter";
+const defaultWorkspaceFilter = "owner:me";
+
+// Function should stay outside components as much as possible; if declared
+// inside the component, React would add/remove event listeners every render
+function subscribeToFilterChanges(notifyReact: () => void) {
+  const onStorageChange = (event: StorageEvent) => {
+    const { key, storageArea, oldValue, newValue } = event;
+
+    const shouldNotify =
+      key === workspaceFilterKey &&
+      storageArea === window.localStorage &&
+      newValue !== oldValue;
+
+    if (shouldNotify) {
+      notifyReact();
+    }
+  };
+
+  window.addEventListener("storage", onStorageChange);
+  return () => window.removeEventListener("storage", onStorageChange);
+}
+
 type UseWorkspacesFilterOptions = {
   searchParamsResult: ReturnType<typeof useSearchParams>;
-  pagination: ReturnType<typeof usePagination>;
+  onPageChange: () => void;
 };
 
-const filterPreferencesKey = "WorkspacesPage/filterPreferences";
-
 const useWorkspacesFilter = ({
   searchParamsResult,
-  pagination,
+  onPageChange,
 }: UseWorkspacesFilterOptions) => {
+  // Using useSyncExternalStore store to safely access localStorage from the
+  // first render; both snapshot callbacks return primitives, so no special
+  // trickery needed to prevent hook from immediately blowing up in dev mode
+  const localStorageFilter = useSyncExternalStore(
+    subscribeToFilterChanges,
+    () => {
+      return (
+        window.localStorage.getItem(workspaceFilterKey) ??
+        defaultWorkspaceFilter
+      );
+    },
+    () => defaultWorkspaceFilter,
+  );
+
   const filter = useFilter({
+    /**
+     * @todo Rename initialValue to fallbackFilter once changes have been tested
+     */
+    initialValue: localStorageFilter,
     searchParamsResult,
-    initialValue: () => {
-      const fallbackValue = "owner:me";
-      if (typeof window === "undefined") {
-        return fallbackValue;
-      }
-
-      return window.localStorage.getItem(filterPreferencesKey) ?? fallbackValue;
-    },
     onUpdate: (newValues) => {
-      window.localStorage.setItem(filterPreferencesKey, newValues);
-      pagination.goToPage(1);
+      window.localStorage.setItem(workspaceFilterKey, newValues);
+      onPageChange();
     },
   });
 
