feat: store tags, block type, and problem type in the URL search string

These search fields store an Array of values, and so this change adds
another hook called useListHelpers to assist with the parsing and
validating of an Array of Typed values.

This feature also revealed two bugs fixed in useStateWithUrlSearchParam:

1. When the returnSetter is called with a function instead of a simple
   value, we need to pass in previous returnValue to the function so it
   can generate the new value.

2. When the returnSetter is called multiple times by a single callback
   (like with clearFilters), the latest changes to the UrlSearchParams
   weren't showing up.

   To fix this, we had to use the location.search string as the "latest"
   previous url search, not the prevParams passed into setSearchParams,
   because these params may not have the latest updates.

src/hooks.ts

@@ -85,10 +85,13 @@ export const useLoadOnScroll = (
 };
 
 /**
- * Hook which stores state variables in the URL search parameters.
- *
- * It wraps useState with functions that get/set a query string
- * search parameter when returning/setting the state variable.
+ * Types used by the useListHelpers and useStateWithUrlSearchParam hooks.
+ */
+export type FromStringFn<Type> = (value: string | null) => Type | undefined;
+export type ToStringFn<Type> = (value: Type | undefined) => string | undefined;
+
+/**
+ * Hook that stores/retrieves state variables using the URL search parameters.
  *
  * @param defaultValue: Type
  *   Returned when no valid value is found in the url search parameter.
@@ -101,26 +104,87 @@ export const useLoadOnScroll = (
 export function useStateWithUrlSearchParam<Type>(
   defaultValue: Type,
   paramName: string,
-  fromString: (value: string | null) => Type | undefined,
-  toString: (value: Type) => string | undefined,
+  fromString: FromStringFn<Type>,
+  toString: ToStringFn<Type>,
 ): [value: Type, setter: Dispatch<SetStateAction<Type>>] {
+  const location = useLocation();
   const [searchParams, setSearchParams] = useSearchParams();
   const returnValue: Type = fromString(searchParams.get(paramName)) ?? defaultValue;
-  // Function to update the url search parameter
-  const returnSetter: Dispatch<SetStateAction<Type>> = useCallback((value: Type) => {
-    setSearchParams((prevParams) => {
-      const paramValue: string = toString(value) ?? '';
-      const newSearchParams = new URLSearchParams(prevParams);
-      // If using the default paramValue, remove it from the search params.
-      if (paramValue === defaultValue) {
+
+  // Update the url search parameter using:
+  type ReturnSetterParams = (
+    // a Type value
+    value?: Type
+    // or a function that returns a Type from the previous returnValue
+    | ((value: Type) => Type)
+  ) => void;
+  const returnSetter: Dispatch<SetStateAction<Type>> = useCallback<ReturnSetterParams>((value) => {
+    setSearchParams((/* prev */) => {
+      const useValue = value instanceof Function ? value(returnValue) : value;
+      const paramValue = toString(useValue);
+
+      // We have to parse the current location.search instead of using prev
+      // in case we call returnSetter multiple times in the same hook
+      // (like clearFilters does).
+      // cf https://github.com/remix-run/react-router/issues/9757
+      const newSearchParams = new URLSearchParams(location.search);
+
+      // If the provided value was invalid (toString returned undefined)
+      // or the same as the defaultValue, remove it from the search params.
+      if (paramValue === undefined || paramValue === defaultValue) {
         newSearchParams.delete(paramName);
       } else {
         newSearchParams.set(paramName, paramValue);
       }
       return newSearchParams;
     }, { replace: true });
-  }, [setSearchParams]);
+  }, [returnValue, setSearchParams]);
 
   // Return the computed value and wrapped set state function
   return [returnValue, returnSetter];
 }
+
+/**
+ * Helper hook for useStateWithUrlSearchParam<Type[]>.
+ *
+ * useListHelpers provides toString and fromString handlers that can:
+ * - split/join a list of values using a separator string, and
+ * - validate each value using the provided functions, omitting any invalid values.
+ *
+ * @param fromString
+ *   Serialize a string to a Type, or undefined if not valid.
+ * @param toString
+ *   Deserialize a Type to a string.
+ * @param separator : string to use when splitting/joining the types.
+ *   Defaults value is ','.
+ */
+export function useListHelpers<Type>({
+  fromString,
+  toString,
+  separator = ',',
+}: {
+  fromString: FromStringFn<Type>,
+  toString: ToStringFn<Type>,
+  separator?: string;
+}): [ FromStringFn<Type[]>, ToStringFn<Type[]> ] {
+  const isType = (item: Type | undefined): item is Type => item !== undefined;
+
+  // Split the given string with separator,
+  // and convert the parts to a list of Types, omiting any invalid Types.
+  const fromStringToList : FromStringFn<Type[]> = (value: string) => (
+    value
+      ? value.split(separator).map(fromString).filter(isType)
+      : []
+  );
+  // Convert an array of Types to strings and join with separator.
+  // Returns undefined if the given list contains no valid Types.
+  const fromListToString : ToStringFn<Type[]> = (value: Type[]) => {
+    const stringValues = value.map(toString).filter((val) => val !== undefined);
+    return (
+      stringValues && stringValues.length
+        ? stringValues.join(separator)
+        : undefined
+    );
+  };
+  return [fromStringToList, fromListToString];
+}

src/search-manager/SearchManager.ts

@@ -12,7 +12,35 @@ import {
   CollectionHit, ContentHit, SearchSortOption, forceArray,
 } from './data/api';
 import { useContentSearchConnection, useContentSearchResults } from './data/apiHooks';
-import { useStateWithUrlSearchParam } from '../hooks';
+import {
+  type FromStringFn,
+  type ToStringFn,
+  useListHelpers,
+  useStateWithUrlSearchParam,
+} from '../hooks';
+
+/**
+ * Typed hook that returns useState if skipUrlUpdate,
+ * or useStateWithUrlSearchParam if it's not.
+ *
+ * Provided here to reduce some code overhead in SearchManager.
+ */
+function useStateOrUrlSearchParam<Type>(
+  defaultValue: Type,
+  paramName: string,
+  fromString: FromStringFn<Type>,
+  toString: ToStringFn<Type>,
+  skipUrlUpdate?: boolean,
+): [value: Type, setter: React.Dispatch<React.SetStateAction<Type>>] {
+  const useStateManager = React.useState<Type>(defaultValue);
+  const urlStateManager = useStateWithUrlSearchParam<Type>(
+    defaultValue,
+    paramName,
+    fromString,
+    toString,
+  );
+  return skipUrlUpdate ? useStateManager : urlStateManager;
+}
 
 export interface SearchContextData {
   client?: MeiliSearch;
@@ -59,50 +87,84 @@ export const SearchContextProvider: React.FC<{
 }) => {
   // Search parameters can be set via the query string
   // E.g. q=draft+text
-  // TODO -- how to scrub search terms?
-  const keywordStateManager = React.useState('');
-  const keywordUrlStateManager = useStateWithUrlSearchParam<string>(
+  // TODO -- how to sanitize search terms?
+  const [searchKeywords, setSearchKeywords] = useStateOrUrlSearchParam<string>(
     '',
     'q',
     (value: string) => value || '',
     (value: string) => value || '',
+    skipUrlUpdate,
+  );
+
+  // Block/problem types can be alphanumeric with underscores or dashes
+  const sanitizeType = (value: string | null | undefined): string | undefined => (
+    (value && /^[a-z0-9_-]+$/.test(value))
+      ? value
+      : undefined
+  );
+  const [typeToList, listToType] = useListHelpers<string>({
+    toString: sanitizeType,
+    fromString: sanitizeType,
+    separator: '|',
+  });
+  const [blockTypesFilter, setBlockTypesFilter] = useStateOrUrlSearchParam<string[]>(
+    [],
+    'bt',
+    typeToList,
+    listToType,
+    skipUrlUpdate,
+  );
+  const [problemTypesFilter, setProblemTypesFilter] = useStateOrUrlSearchParam<string[]>(
+    [],
+    'pt',
+    typeToList,
+    listToType,
+    skipUrlUpdate,
   );
-  const [searchKeywords, setSearchKeywords] = (
-    skipUrlUpdate
-      ? keywordStateManager
-      : keywordUrlStateManager
+
+  // Tags can be almost any string value, except our separator (|)
+  // TODO how to sanitize tags?
+  const sanitizeTag = (value: string | null | undefined): string | undefined => (
+    (value && /^[^|]+$/.test(value))
+      ? value
+      : undefined
+  );
+  const [tagToList, listToTag] = useListHelpers<string>({
+    toString: sanitizeTag,
+    fromString: sanitizeTag,
+    separator: '|',
+  });
+  const [tagsFilter, setTagsFilter] = useStateOrUrlSearchParam<string[]>(
+    [],
+    'tg',
+    tagToList,
+    listToTag,
+    skipUrlUpdate,
   );
 
-  const [blockTypesFilter, setBlockTypesFilter] = React.useState<string[]>([]);
-  const [problemTypesFilter, setProblemTypesFilter] = React.useState<string[]>([]);
-  const [tagsFilter, setTagsFilter] = React.useState<string[]>([]);
-  const [usageKey, setUsageKey] = useStateWithUrlSearchParam(
+  const [usageKey, setUsageKey] = useStateOrUrlSearchParam<string>(
     '',
     'usageKey',
+    // TODO should sanitize usageKeys too.
     (value: string) => value,
     (value: string) => value,
+    skipUrlUpdate,
   );
 
   let extraFilter: string[] = forceArray(props.extraFilter);
   if (usageKey) {
     extraFilter = union(extraFilter, [`usage_key = "${usageKey}"`]);
   }
 
-  // The search sort order can be set via the query string
-  // E.g. ?sort=display_name:desc maps to SearchSortOption.TITLE_ZA.
   // Default sort by Most Relevant if there's search keyword(s), else by Recently Modified.
   const defaultSearchSortOrder = searchKeywords ? SearchSortOption.RELEVANCE : SearchSortOption.RECENTLY_MODIFIED;
-  let sortStateManager = React.useState<SearchSortOption>(defaultSearchSortOrder);
-  const sortUrlStateManager = useStateWithUrlSearchParam<SearchSortOption>(
+  const [searchSortOrder, setSearchSortOrder] = useStateOrUrlSearchParam<SearchSortOption>(
     defaultSearchSortOrder,
     'sort',
     (value: string) => Object.values(SearchSortOption).find((enumValue) => value === enumValue),
     (value: SearchSortOption) => value.toString(),
+    skipUrlUpdate,
   );
-  if (!skipUrlUpdate) {
-    sortStateManager = sortUrlStateManager;
-  }
-  const [searchSortOrder, setSearchSortOrder] = sortStateManager;
   // SearchSortOption.RELEVANCE is special, it means "no custom sorting", so we
   // send it to useContentSearchResults as an empty array.
   const searchSortOrderToUse = overrideSearchSortOrder ?? searchSortOrder;