fix: recompute based on key

src/use-query.spec.ts

@@ -1,10 +1,18 @@
-import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
+import {
+  describe,
+  it,
+  expect,
+  vi,
+  beforeEach,
+  afterEach,
+  MockInstance,
+} from 'vitest'
 import { UseQueryOptions, useQuery } from './use-query'
 import { mount } from '@vue/test-utils'
 import { createPinia } from 'pinia'
-import { defineComponent } from 'vue'
+import { defineComponent, nextTick, ref } from 'vue'
 import { GlobalMountOptions } from 'node_modules/@vue/test-utils/dist/types'
-import { delay, runTimers } from '../test/utils'
+import { delay, isSpy, runTimers } from '../test/utils'
 import { useDataFetchingStore } from './data-fetching-store'
 import { entryNodeSize } from './tree-map'
 
@@ -210,19 +218,121 @@ describe('useQuery', () => {
     })
   })
 
-  describe.skip('refresh', () => {
-    it('refreshes the data', async () => {
-      const { wrapper, fetcher } = mountSimple()
+  describe('refresh data', () => {
+    const mountDynamicKey = <TResult = { id: number; when: number }>(
+      options: Partial<UseQueryOptions<TResult>> & { initialId?: number } = {},
+      mountOptions?: GlobalMountOptions
+    ) => {
+      let fetcher!: MockInstance
+
+      const wrapper = mount(
+        defineComponent({
+          render: () => null,
+          setup() {
+            const id = ref(options.initialId ?? 0)
+            fetcher = options.fetcher
+              ? isSpy(options.fetcher)
+                ? options.fetcher
+                : vi.fn(options.fetcher)
+              : vi.fn(async () => {
+                  await delay(0)
+                  return { id: id.value, when: Date.now() }
+                })
+
+            return {
+              id,
+              async setId(newId: number) {
+                id.value = newId
+                // awaits the delay of 0
+                await runTimers()
+                // renders again
+                await nextTick()
+              },
+              ...useQuery<TResult>({
+                key: () => ['data', id.value],
+                ...options,
+                // @ts-expect-error: generic unmatched but types work
+                fetcher,
+              }),
+            }
+          },
+        }),
+        {
+          global: {
+            plugins: [createPinia()],
+            ...mountOptions,
+          },
+        }
+      )
+      return Object.assign([wrapper, fetcher] as const, {
+        wrapper,
+        fetcher,
+      })
+    }
+
+    it('refreshes the data if mounted and the key changes', async () => {
+      const { wrapper, fetcher } = mountDynamicKey({
+        initialId: 0,
+      })
 
       await runTimers()
-      expect(wrapper.vm.data).toBe(42)
+      expect(wrapper.vm.data?.id).toBe(0)
       expect(fetcher).toHaveBeenCalledTimes(1)
 
-      mountSimple()
+      await wrapper.vm.setId(1)
+
+      expect(fetcher).toHaveBeenCalledTimes(2)
+      expect(wrapper.vm.data?.id).toBe(1)
+    })
+
+    it('avoids a new fetch if the key changes but the data is not stale', async () => {
+      const { wrapper, fetcher } = mountDynamicKey({
+        initialId: 0,
+        staleTime: 1000,
+      })
 
       await runTimers()
+      expect(wrapper.vm.data?.id).toBe(0)
+      expect(fetcher).toHaveBeenCalledTimes(1)
+
+      await wrapper.vm.setId(1)
+      await wrapper.vm.setId(0)
+
+      expect(fetcher).toHaveBeenCalledTimes(2)
+    })
+
+    it('does not refresh by default when mounting a new component that uses the same key', async () => {
+      const pinia = createPinia()
+      const fetcher = vi.fn().mockResolvedValue({ id: 0, when: Date.now() })
+      mountDynamicKey({ initialId: 0, fetcher }, { plugins: [pinia] })
+      await runTimers()
+      expect(fetcher).toHaveBeenCalledTimes(1)
+
+      mountDynamicKey({ initialId: 0 }, { plugins: [pinia] })
+      await runTimers()
+      // not called because data is fresh
+      expect(fetcher).toHaveBeenCalledTimes(1)
+    })
+
+    it('refreshes when mounting a new component that uses the same key if data is stale', async () => {
+      const pinia = createPinia()
+      const fetcher = vi.fn().mockResolvedValue({ id: 0, when: Date.now() })
+      mountDynamicKey(
+        // staleTime doesn't matter here
+        { initialId: 0, staleTime: 10, fetcher },
+        { plugins: [pinia] }
+      )
+      await runTimers()
+      expect(fetcher).toHaveBeenCalledTimes(1)
+      await vi.advanceTimersByTime(100)
+
+      mountDynamicKey(
+        { initialId: 0, staleTime: 10, fetcher },
+        { plugins: [pinia] }
+      )
+      await runTimers()
+      // called because data is stale
       expect(fetcher).toHaveBeenCalledTimes(2)
-      expect(wrapper.vm.data).toBe(42)
     })
   })
 

src/use-query.ts

@@ -15,6 +15,8 @@ import {
   ShallowRef,
   Ref,
   MaybeRefOrGetter,
+  watch,
+  getCurrentInstance,
 } from 'vue'
 import {
   UseQueryPropertiesEntry,
@@ -29,6 +31,11 @@ export interface UseQueryReturn<TResult = unknown, TError = Error>
   extends UseQueryStateEntry<TResult, TError>,
     Pick<UseQueryPropertiesEntry<TResult, TError>, 'refresh' | 'refetch'> {}
 
+/**
+ * `true` refetch if data is stale, false never refetch, 'always' always refetch.
+ */
+export type _RefetchOnControl = boolean | 'always'
+
 /**
  * Key used to identify a query.
  */
@@ -58,8 +65,9 @@ export interface UseQueryOptions<TResult = unknown> {
 
   initialData?: () => TResult
   // TODO: rename to refresh and use refresh instead by default?
-  refetchOnWindowFocus?: boolean // TODO: | 'force' or options to adapt this
-  refetchOnReconnect?: boolean
+  refetchOnMount?: _RefetchOnControl
+  refetchOnWindowFocus?: _RefetchOnControl
+  refetchOnReconnect?: _RefetchOnControl
 }
 
 /**
@@ -69,8 +77,9 @@ export const USE_QUERY_DEFAULTS = {
   staleTime: 1000 * 5, // 5 seconds
   gcTime: 1000 * 60 * 5, // 5 minutes
   // avoid type narrowing to `true`
-  refetchOnWindowFocus: true as UseQueryOptions['refetchOnWindowFocus'],
-  refetchOnReconnect: true as UseQueryOptions['refetchOnReconnect'],
+  refetchOnWindowFocus: true as _RefetchOnControl,
+  refetchOnReconnect: true as _RefetchOnControl,
+  refetchOnMount: true as _RefetchOnControl,
 } satisfies Partial<UseQueryOptions>
 // TODO: inject for the app rather than a global variable
 
@@ -102,11 +111,39 @@ export function useQuery<TResult, TError = Error>(
       queryReturn.isPending.value
   })
 
+  const hasCurrentInstance = getCurrentInstance()
+
+  // should we be watching entry
+  let isActive = false
+  if (hasCurrentInstance) {
+    onMounted(() => {
+      isActive = true
+    })
+  } else {
+    isActive = true
+    entry.value.refresh()
+  }
+
+  watch(entry, (entry, _, onCleanup) => {
+    if (!isActive) return
+    entry.refresh()
+    onCleanup(() => {
+      // TODO: decrement ref count
+    })
+  })
+
   // only happens on client
   // we could also call fetch instead but forcing a refresh is more interesting
-  onMounted(entry.value.refresh)
-  // TODO: optimize so it doesn't refresh if we are hydrating
-
+  if (options.refetchOnMount && hasCurrentInstance) {
+    // TODO: optimize so it doesn't refresh if we are hydrating
+    onMounted(() => {
+      if (options.refetchOnMount === 'always') {
+        entry.value.refetch()
+      } else {
+        entry.value.refresh()
+      }
+    })
+  }
   // TODO: we could save the time it was fetched to avoid fetching again. This is useful to not refetch during SSR app but do refetch in SSG apps if the data is stale. Careful with timers and timezones
 
   onScopeDispose(() => {
@@ -117,28 +154,35 @@ export function useQuery<TResult, TError = Error>(
     if (options.refetchOnWindowFocus) {
       useEventListener(document, 'visibilitychange', () => {
         if (document.visibilityState === 'visible') {
-          entry.value.refetch()
+          if (options.refetchOnWindowFocus === 'always') {
+            entry.value.refetch()
+          } else {
+            entry.value.refresh()
+          }
         }
       })
     }
 
     if (options.refetchOnReconnect) {
       useEventListener(window, 'online', () => {
-        entry.value.refetch()
+        if (options.refetchOnReconnect === 'always') {
+          entry.value.refetch()
+        } else {
+          entry.value.refresh()
+        }
       })
     }
   }
 
   // TODO: handle if key is reactive
 
   const queryReturn = {
-    data: entry.value.data,
-    error: entry.value.error,
-    isFetching: entry.value.isFetching,
-    isPending: entry.value.isPending,
-    status: entry.value.status,
+    data: computed(() => entry.value.data.value),
+    error: computed(() => entry.value.error.value),
+    isFetching: computed(() => entry.value.isFetching.value),
+    isPending: computed(() => entry.value.isPending.value),
+    status: computed(() => entry.value.status.value),
 
-    // TODO: do we need to force bound to the entry?
     refresh: () => entry.value.refresh(),
     refetch: () => entry.value.refetch(),
   } satisfies UseQueryReturn<TResult, TError>

test/utils.ts

@@ -1,4 +1,4 @@
-import { vi } from 'vitest'
+import { Mock, vi } from 'vitest'
 import { nextTick } from 'vue'
 
 export const delay = (ms: number) => new Promise((r) => setTimeout(r, ms))
@@ -12,3 +12,12 @@ export const runTimers = async (onlyPending = true) => {
   }
   await nextTick()
 }
+
+export function isSpy(fn: any): fn is Mock {
+  return (
+    typeof fn === 'function' &&
+    'mock' in fn &&
+    typeof fn.mock === 'object' &&
+    Array.isArray(fn.mock.calls)
+  )
+}