add shared hook for resetting state when a key changes

src/components/MyObservations/MyObservationsGroupedByIconicTaxaView.tsx

@@ -13,8 +13,7 @@ import React, {
   useCallback, useEffect, useMemo, useRef, useState,
 } from "react";
 import { ICONIC_TAXA_GROUP, iconicTaxaGroupIcon } from "sharedHelpers/iconicTaxaGroupOrder";
-import type { OBSERVATIONS_SORT } from "sharedHelpers/observationsSort";
-import { useCurrentUser, useTranslation } from "sharedHooks";
+import { useCurrentUser, useStateResetOn, useTranslation } from "sharedHooks";
 
 import type {
   IconicTaxaHeader,
@@ -56,13 +55,6 @@ interface Props {
   listHeaderContent?: React.ReactElement | null;
 }
 
-// Which categories the user has collapsed, tracked alongside the sort they were collapsed
-// under so changing sort reopens everything without an effect
-interface CollapseState {
-  sortBy: OBSERVATIONS_SORT;
-  categories: Set<ICONIC_TAXA_GROUP>;
-}
-
 const NONE_COLLAPSED: Set<ICONIC_TAXA_GROUP> = new Set( );
 
 // How many tiles ahead of the last loaded one to start fetching.
@@ -87,13 +79,11 @@ const MyObservationsGroupedByIconicTaxaView = ( {
   } = useIconicTaxaObservationCounts( );
   const unsyncedByCategory = useUnsyncedObservationIdsByIconicTaxon( );
 
-  const [collapseState, setCollapseState] = useState<CollapseState>( {
-    sortBy: observationsSort,
-    categories: NONE_COLLAPSED,
-  } );
-  const collapsedCategories = collapseState.sortBy === observationsSort
-    ? collapseState.categories
-    : NONE_COLLAPSED;
+  // Changing sort reopens every section, since the list they were collapsed against is gone
+  const [collapsedCategories, setCollapsedCategories] = useStateResetOn(
+    observationsSort,
+    NONE_COLLAPSED,
+  );
 
   const {
     sections,
@@ -166,7 +156,7 @@ const MyObservationsGroupedByIconicTaxaView = ( {
     } else {
       categories.delete( category );
     }
-    setCollapseState( { sortBy: observationsSort, categories } );
+    setCollapsedCategories( categories );
     if ( !isCollapsing ) return;
 
     const headerRow = rows.findIndex(
@@ -177,7 +167,7 @@ const MyObservationsGroupedByIconicTaxaView = ( {
     }
 
     advanceFrontier( );
-  }, [advanceFrontier, collapsedCategories, observationsSort, rows] );
+  }, [advanceFrontier, collapsedCategories, rows, setCollapsedCategories] );
 
   useEffect( ( ) => {
     const index = pinHeaderRowRef.current;

src/components/MyObservations/hooks/useIconicTaxaSectionObservations.ts

@@ -3,14 +3,14 @@ import { searchObservations } from "api/observations";
 import { getJWT } from "components/LoginSignUp/AuthenticationService";
 import type { IconicTaxaSectionState } from "components/MyObservations/helpers/iconicTaxaSections";
 import { RealmContext } from "providers/contexts";
-import { useCallback, useMemo, useState } from "react";
+import { useCallback, useMemo } from "react";
 import Observation from "realmModels/Observation";
 import type { ICONIC_TAXA_GROUP, IconicTaxaGroupCount } from "sharedHelpers/iconicTaxaGroupOrder";
 import { log } from "sharedHelpers/logger";
 import { handleRetryDelay, reactQueryRetry } from "sharedHelpers/logging";
 import type { OBSERVATIONS_SORT } from "sharedHelpers/observationsSort";
 import { observationSortToApiParams } from "sharedHelpers/observationsSort";
-import { useCurrentUser } from "sharedHooks";
+import { useCurrentUser, useStateResetOn } from "sharedHooks";
 
 const { useRealm } = RealmContext;
 
@@ -33,12 +33,7 @@ interface IconicTaxonPage {
 // so this doubles as the activation frontier.
 type PagesByCategory = Partial<Record<ICONIC_TAXA_GROUP, number>>;
 
-// Pages are tracked alongside the sort they were fetched under, so changing sort discards them
-// on the next render
-interface PageState {
-  sortKey: string;
-  pages: PagesByCategory;
-}
+const NOTHING_REQUESTED: PagesByCategory = {};
 
 interface Params {
   collapsedCategories: Set<ICONIC_TAXA_GROUP>;
@@ -84,27 +79,23 @@ const useIconicTaxaSectionObservations = ( {
   const sortParams = useMemo( ( ) => observationSortToApiParams( sortBy ), [sortBy] );
   const sortKey = `${sortParams.order_by}-${sortParams.order}`;
 
-  const [pageState, setPageState] = useState<PageState>( { sortKey, pages: {} } );
+  // Changing sort invalidates every page, so this starts over rather than re-requesting them
+  // all under the new order
+  const [requestedPages, setPages] = useStateResetOn( sortKey, NOTHING_REQUESTED );
 
+  // Before the user has asked for anything, the most-observed category is treated as already
+  // requested so it starts loading on this render; everything below it waits for them to
+  // scroll or collapse their way down. Seeded rather than written back, so requestedPages
+  // stays honest about what the user has actually asked for. Categories the server has nothing
+  // for are skipped: their header still renders and will show any locally-saved observations,
+  // but there's nothing to request.
   const pagesByCategory = useMemo( ( ) => {
-    const pages = pageState.sortKey === sortKey
-      ? pageState.pages
-      : {};
-    if ( Object.keys( pages ).length > 0 ) return pages;
-    // Nothing requested yet, either because the view just opened or because the sort changed.
-    // Seed the most-observed category so it starts loading on this render; everything below it
-    // waits for the user to scroll or collapse their way down. Categories the server has
-    // nothing for are skipped: their header still renders and will show any locally-saved
-    // observations, but there's nothing to request.
+    if ( Object.keys( requestedPages ).length > 0 ) return requestedPages;
     const first = orderedCounts.find( ( { count } ) => count > 0 );
     return first
       ? { [first.category]: 1 }
-      : pages;
-  }, [orderedCounts, pageState, sortKey] );
-
-  const setPages = useCallback( ( pages: PagesByCategory ) => {
-    setPageState( { sortKey, pages } );
-  }, [sortKey] );
+      : requestedPages;
+  }, [orderedCounts, requestedPages] );
 
   const descriptors = useMemo( ( ) => orderedCounts.flatMap( ( { category } ) => {
     const highestPage = pagesByCategory[category] ?? 0;

src/sharedHooks/index.ts

@@ -25,6 +25,7 @@ export { default as useQuery } from "./useQuery";
 export { default as useRemoteObservation } from "./useRemoteObservation";
 export { default as useScrollToOffset } from "./useScrollToOffset";
 export { default as useShare } from "./useShare";
+export { default as useStateResetOn } from "./useStateResetOn";
 export { default as useStoredLayout } from "./useStoredLayout";
 export { default as useSuggestions } from "./useSuggestions/useSuggestions";
 export { default as useTaxon } from "./useTaxon";

src/sharedHooks/useStateResetOn.ts

@@ -0,0 +1,30 @@
+import { useCallback, useState } from "react";
+
+// State that goes back to its initial value whenever `key` changes, without an effect and
+// without the extra render an effect would cost — the reset happens during the render where
+// the new key first appears.
+//
+// `initial` must be a stable reference since it gets returned after a key change. A fresh object
+// every render would defeat memoization in everything downstream.
+//
+// The value is stored against the key it was written under, so returning to an earlier key
+// restores what was set then rather than the initial value. Only the most recent write is
+// kept, so this is a one-slot memory, not a history.
+const useStateResetOn = <T, >(
+  key: unknown,
+  initial: T,
+): [T, ( next: T ) => void] => {
+  const [state, setState] = useState<{ key: unknown; value: T }>( { key, value: initial } );
+
+  const value = state.key === key
+    ? state.value
+    : initial;
+
+  const setValue = useCallback( ( next: T ) => {
+    setState( { key, value: next } );
+  }, [key] );
+
+  return [value, setValue];
+};
+
+export default useStateResetOn;

tests/unit/sharedHooks/useStateResetOn.test.js

@@ -0,0 +1,68 @@
+import { act, renderHook } from "@testing-library/react-native";
+import useStateResetOn from "sharedHooks/useStateResetOn";
+
+const INITIAL = { open: false };
+
+const renderStateHook = ( ) => renderHook(
+  ( { key } ) => useStateResetOn( key, INITIAL ),
+  { initialProps: { key: "a" } },
+);
+
+describe( "useStateResetOn", ( ) => {
+  it( "keeps what was set while the key is unchanged", ( ) => {
+    const { rerender, result } = renderStateHook( );
+
+    act( ( ) => result.current[1]( { open: true } ) );
+    rerender( { key: "a" } );
+
+    expect( result.current[0] ).toEqual( { open: true } );
+  } );
+
+  it( "goes back to the initial value when the key changes", ( ) => {
+    const { rerender, result } = renderStateHook( );
+    act( ( ) => result.current[1]( { open: true } ) );
+
+    rerender( { key: "b" } );
+
+    expect( result.current[0] ).toBe( INITIAL );
+  } );
+
+  it( "resets during the render the new key arrives in, not a render later", ( ) => {
+    const values = [];
+    const { rerender } = renderHook(
+      ( { key } ) => {
+        const [value] = useStateResetOn( key, INITIAL );
+        values.push( value );
+        return null;
+      },
+      { initialProps: { key: "a" } },
+    );
+
+    rerender( { key: "b" } );
+
+    // an effect-based reset would show the stale value once before correcting it
+    expect( values.every( value => value === INITIAL ) ).toBe( true );
+  } );
+
+  it( "restores what was set under a key when that key comes back", ( ) => {
+    const { rerender, result } = renderStateHook( );
+    act( ( ) => result.current[1]( { open: true } ) );
+
+    rerender( { key: "b" } );
+    expect( result.current[0] ).toBe( INITIAL );
+
+    rerender( { key: "a" } );
+    expect( result.current[0] ).toEqual( { open: true } );
+  } );
+
+  it( "keeps only the most recent write, so it is one slot rather than a history", ( ) => {
+    const { rerender, result } = renderStateHook( );
+    act( ( ) => result.current[1]( { open: true } ) );
+
+    rerender( { key: "b" } );
+    act( ( ) => result.current[1]( { open: false } ) );
+    rerender( { key: "a" } );
+
+    expect( result.current[0] ).toBe( INITIAL );
+  } );
+} );