more hook fixes.

src/components/ga4/DimensionsMetricsExplorer/useDimensionsAndMetrics.ts

@@ -85,5 +85,5 @@ export const useDimensionsAndMetrics = (
         }
       }
     }
-  }, [])
+  }, [metadataRequest])
 }

src/hooks/index.spec.ts

@@ -2,12 +2,9 @@ import "@testing-library/jest-dom"
 import { renderHook } from "@testing-library/react-hooks"
 
 import { StorageKey } from "@/constants"
-import { useCallback, useMemo } from "react"
-import useCached from "./useCached"
-import moment from "moment"
+import { useState } from "react"
 import { act } from "react-test-renderer"
 import { usePersistantObject } from "."
-import { RequestStatus, successful } from "@/types"
 
 describe("usePersistantObject", () => {
   beforeEach(() => {
@@ -28,20 +25,22 @@ describe("usePersistantObject", () => {
       window.localStorage.setItem(firstKey, JSON.stringify(firstValue))
       window.localStorage.setItem(secondKey, JSON.stringify(secondValue))
 
-      const { result, rerender } = renderHook(
-        ({ key }: { key: StorageKey }) => {
-          return usePersistantObject(key)
+      const { result } = renderHook(
+        () => {
+          const [key, setKey] = useState(firstKey)
+          const sut = usePersistantObject(key)
+          return { sut, setKey }
         },
         { initialProps: { key: firstKey } }
       )
 
-      expect(result.current[0]).toEqual(firstValue)
+      expect(result.current.sut[0]).toEqual(firstValue)
 
       act(() => {
-        rerender({ key: secondKey })
+        result.current.setKey(secondKey)
       })
 
-      expect(result.current[0]).toEqual(secondValue)
+      expect(result.current.sut[0]).toEqual(secondValue)
     })
   })
 })

src/hooks/index.ts

@@ -54,11 +54,14 @@ export const usePersistentString: UsePersistentString = (
   initialValue,
   overwrite
 ) => {
+  if (IS_SSR) {
+    return useState(initialValue)
+  }
   const [value, setValue] = useState<string | undefined>(() => {
     if (overwrite !== undefined) {
       return overwrite
     }
-    const fromStorage = IS_SSR ? null : window.localStorage.getItem(key)
+    const fromStorage = window.localStorage.getItem(key)
     if (fromStorage === null) {
       return initialValue
     }
@@ -89,7 +92,7 @@ const getObjectFromLocalStorage = <T>(
   if (IS_SSR) {
     return undefined
   }
-  let asString = IS_SSR ? null : window.localStorage.getItem(key)
+  let asString = window.localStorage.getItem(key)
   if (asString === null || asString === "undefined") {
     return defaultValue
   }
@@ -100,6 +103,10 @@ export const usePersistantObject = <T extends {}>(
   key: StorageKey,
   defaultValue?: T
 ): [T | undefined, Dispatch<T | undefined>] => {
+  if (IS_SSR) {
+    return useState<T | undefined>(defaultValue)
+  }
+
   const [localValue, setValueLocal] = useState(() => {
     return getObjectFromLocalStorage(key, defaultValue)
   })
@@ -122,8 +129,11 @@ export const usePersistantObject = <T extends {}>(
     [key]
   )
 
+  // Note - This feels wrong, but I have to depend on localValue changing to
+  // catch changes that only happen when setValue is run. Don't remove
+  // localValue
   const fromStorage = React.useMemo(
-    () => getObjectFromLocalStorage(key, defaultValue) || localValue,
+    () => getObjectFromLocalStorage(key, defaultValue),
     [key, localValue, defaultValue]
   )
 

src/hooks/useCached.spec.ts

@@ -2,11 +2,11 @@ import "@testing-library/jest-dom"
 import { renderHook } from "@testing-library/react-hooks"
 
 import { StorageKey } from "@/constants"
-import { useCallback, useMemo } from "react"
+import { useCallback, useMemo, useState } from "react"
 import useCached from "./useCached"
 import moment from "moment"
 import { act } from "react-test-renderer"
-import { RequestStatus, successful } from "@/types"
+import { inProgress, RequestStatus, successful } from "@/types"
 
 describe("useCached", () => {
   // The specific storage key shouldn't matter.
@@ -154,6 +154,7 @@ describe("useCached", () => {
       expect(actual).not.toBeUndefined()
       expect(actual!.value).toEqual(secondValue)
     })
+
     test("and switch to another value in cache, return other value", async () => {
       const firstKey = "firstKey" as StorageKey
       const firstValue = {
@@ -182,26 +183,71 @@ describe("useCached", () => {
         })
       )
 
-      const makeRequest = async () => {
-        fail("should not be called if value is in localStorage")
+      const { result } = renderHook(() => {
+        const [key, setKey] = useState(firstKey)
+        const makeRequest = useCallback(async () => {
+          return secondValue
+        }, [])
+        const requestReady = useMemo(() => true, [])
+        const sut = useCached(key, makeRequest, expirey, requestReady)
+        return { sut, setKey }
+      })
+
+      expect(successful(result.current.sut)).not.toBeUndefined()
+      expect(successful(result.current.sut)!.value).toEqual(firstValue)
+
+      act(() => {
+        result.current.setKey(secondKey)
+      })
+
+      expect(successful(result.current.sut)).not.toBeUndefined()
+      expect(successful(result.current.sut)!.value).toEqual(secondValue)
+    })
+
+    test("and switch to another value not in cache, re-runs request and sets to InProgress", async () => {
+      const firstKey = "firstKey" as StorageKey
+      const firstValue = {
+        hi: "there",
+      }
+      const secondKey = "secondKey" as StorageKey
+      const secondValue = {
+        alsoHi: "alsoThere",
       }
-      const requestReady = true
-      const { result, rerender } = renderHook(
-        ({ key }: { key: StorageKey }) => {
-          return useCached(key, makeRequest, expirey, requestReady)
-        },
-        { initialProps: { key: firstKey } }
+
+      window.localStorage.setItem(
+        firstKey,
+        JSON.stringify({
+          value: firstValue,
+          "@@_lastFetched": moment.now(),
+          "@@_cacheKey": firstKey,
+        })
       )
 
-      expect(successful(result.current)).not.toBeUndefined()
-      expect(successful(result.current)!.value).toEqual(firstValue)
+      const { result, waitForNextUpdate } = renderHook(() => {
+        const [key, setKey] = useState(firstKey)
+        const makeRequest = useCallback(async () => {
+          return secondValue
+        }, [])
+        const requestReady = useMemo(() => true, [])
+        const sut = useCached(key, makeRequest, expirey, requestReady)
+        return { sut, setKey }
+      })
+
+      expect(successful(result.current.sut)).not.toBeUndefined()
+      expect(successful(result.current.sut)!.value).toEqual(firstValue)
 
       act(() => {
-        rerender({ key: secondKey })
+        result.current.setKey(secondKey)
+      })
+
+      expect(inProgress(result.current.sut)).not.toBeUndefined()
+
+      await act(async () => {
+        await waitForNextUpdate()
       })
 
-      expect(successful(result.current)).not.toBeUndefined()
-      expect(successful(result.current)!.value).toEqual(secondValue)
+      expect(successful(result.current.sut)).not.toBeUndefined()
+      expect(successful(result.current.sut)!.value).toEqual(secondValue)
     })
   })
 })

src/hooks/useCached.ts

@@ -98,13 +98,15 @@ const useCached = <T, E = any>(
         return { status, error }
       }
       case RequestStatus.Successful: {
+        // If the cache is undefined, but the status is Successful, that means
+        // the key was just changed.
         if (cached === undefined) {
-          throw new Error("Invalid invariant, cached must be defined here")
+          return { status: RequestStatus.InProgress }
         }
         return { status, value: cached.value, bustCache }
       }
     }
-  }, [status, cached, bustCache, maxAge])
+  }, [status, cached, bustCache])
 }
 
 export default useCached