refactor: working internals

src/data-fetching-store.ts

@@ -1,52 +1,101 @@
 import { defineStore } from 'pinia'
-import { shallowReactive } from 'vue'
-import type {
-  UseQueryOptionsWithDefaults,
-  UseDataFetchingQueryEntry,
-  UseQueryKey,
-} from './use-query'
+import {
+  type Ref,
+  shallowReactive,
+  getCurrentScope,
+  ShallowRef,
+  ref,
+} from 'vue'
+import type { UseQueryOptionsWithDefaults, UseQueryKey } from './use-query'
+
+export interface UseQueryStateEntry<TResult = unknown, TError = unknown> {
+  // TODO: is it worth to be a shallowRef?
+  data: Ref<TResult | undefined>
+  error: ShallowRef<TError | null>
 
-export const useDataFetchingStore = defineStore('PiniaColada', () => {
   /**
-   * - These are reactive because they are needed for SSR
-   * - They are split into multiple stores to better handle reactivity
-   * - With `shallowReactive()` we only observe the first level of the object, which is enough here as the user only
-   *   gets read-only access to the data
+   * Returns whether the request is still pending its first call
    */
-  const dataRegistry = shallowReactive(new Map<UseQueryKey, unknown>())
-  const errorRegistry = shallowReactive(new Map<UseQueryKey, any>())
-  const isFetchingRegistry = shallowReactive(new Map<UseQueryKey, boolean>())
+  isPending: Ref<boolean>
+  /**
+   * Returns whether the request is currently fetching data
+   */
+  isFetching: Ref<boolean>
+}
 
-  // no reactive on this one as it's only used internally and is not needed for hydration
-  const queryEntriesRegistry = new Map<
+export interface UseQueryPropertiesEntry<TResult = unknown, TError = unknown> {
+  // TODO: should we just have refresh and allow a parameter to force a refresh? instead of having fetch and refresh
+  /**
+   * Refreshes the data ignoring any cache but still decouples the refreshes (only one refresh at a time)
+   * @returns a promise that resolves when the refresh is done
+   */
+  refresh: () => Promise<void>
+  /**
+   * Fetches the data but only if it's not already fetching
+   * @returns a promise that resolves when the refresh is done
+   */
+  fetch: () => Promise<TResult>
+
+  pending: null | {
+    refreshCall: Promise<void>
+    when: number
+  }
+
+  previous: null | {
+    /**
+     * When was this data fetched the last time in ms
+     */
+    when: number
+    data: TResult | undefined
+    error: TError | null
+  }
+}
+
+export interface UseQueryEntry<TResult = unknown, TError = Error>
+  extends UseQueryStateEntry<TResult, TError>,
+    UseQueryPropertiesEntry<TResult, TError> {}
+
+export const useDataFetchingStore = defineStore('PiniaColada', () => {
+  const entryStateRegistry = shallowReactive(
+    new Map<UseQueryKey, UseQueryStateEntry>()
+  )
+  // these are not reactive as they are mostly functions
+  const entryPropertiesRegistry = new Map<
     UseQueryKey,
-    UseDataFetchingQueryEntry<unknown, unknown>
+    UseQueryPropertiesEntry
   >()
 
+  // this allows use to attach reactive effects to the scope later on
+  const scope = getCurrentScope()!
+
+  // no reactive on this one as it's only used internally and is not needed for hydration
+  // const queryEntriesRegistry = new Map<
+  //   UseQueryKey,
+  //   UseDataFetchingQueryEntry<unknown, unknown>
+  // >()
+
   function ensureEntry<TResult = unknown, TError = Error>(
     key: UseQueryKey,
     {
       fetcher,
       initialData: initialValue,
       staleTime: cacheTime,
     }: UseQueryOptionsWithDefaults<TResult>
-  ): UseDataFetchingQueryEntry<TResult, TError> {
-    // ensure the data
+  ): UseQueryEntry<TResult, TError> {
+    // ensure the state
     console.log('⚙️ Ensuring entry', key)
-    if (!dataRegistry.has(key)) {
-      dataRegistry.set(key, initialValue?.() ?? undefined)
-      errorRegistry.set(key, null)
-      isFetchingRegistry.set(key, false)
-    }
+    if (!entryStateRegistry.has(key)) {
+      entryStateRegistry.set(
+        key,
+        scope.run(() => ({
+          data: ref(initialValue?.()),
+          error: ref(null),
+          isPending: ref(false),
+          isFetching: ref(false),
+        }))!
+      )
 
-    // we need to repopulate the entry registry separately from data and errors
-    if (!queryEntriesRegistry.has(key)) {
-      const entry: UseDataFetchingQueryEntry<TResult, TError> = {
-        data: () => dataRegistry.get(key) as TResult,
-        error: () => errorRegistry.get(key) as TError,
-        // FIXME: not reactive
-        isPending: () => !entry.previous,
-        isFetching: () => isFetchingRegistry.get(key)!,
+      const propertiesEntry: UseQueryPropertiesEntry<TResult, TError> = {
         pending: null,
         previous: null,
         async fetch(): Promise<TResult> {
@@ -60,7 +109,7 @@ export const useDataFetchingStore = defineStore('PiniaColada', () => {
             await (entry.pending?.refreshCall ?? entry.refresh())
           }
 
-          return entry.data()!
+          return entry.data.value!
         },
         async refresh() {
           console.log('🔄 refreshing', key)
@@ -69,45 +118,59 @@ export const useDataFetchingStore = defineStore('PiniaColada', () => {
             console.log('  -> skipped!')
             return entry.pending.refreshCall
           }
-          isFetchingRegistry.set(key, true)
-          errorRegistry.set(key, null)
+          entry.isFetching.value = true
+          entry.error.value = null
           const nextPrevious = {
             when: 0,
             data: undefined as TResult | undefined,
             error: null as TError | null,
-          } satisfies UseDataFetchingQueryEntry['previous']
+          } satisfies UseQueryPropertiesEntry<TResult, TError>['previous']
 
           entry.pending = {
             refreshCall: fetcher()
               .then((data) => {
                 nextPrevious.data = data
-                dataRegistry.set(key, data)
+                entry.data.value = data
               })
               .catch((error) => {
                 nextPrevious.error = error
-                errorRegistry.set(key, error)
+                entry.error.value = error
                 throw error
               })
               .finally(() => {
                 entry.pending = null
                 nextPrevious.when = Date.now()
                 entry.previous = nextPrevious
-                isFetchingRegistry.set(key, false)
+                entry.isFetching.value = false
+                entry.isPending.value = false
               }),
             when: Date.now(),
           }
 
           return entry.pending.refreshCall
         },
       }
-      queryEntriesRegistry.set(key, entry)
+      entryPropertiesRegistry.set(key, propertiesEntry)
+    }
+
+    const stateEntry = entryStateRegistry.get(key)! as UseQueryStateEntry<
+      TResult,
+      TError
+    >
+    const propertiesEntry = entryPropertiesRegistry.get(
+      key
+    )! as UseQueryPropertiesEntry<TResult, TError>
+
+    const entry = {
+      ...stateEntry,
+      ...propertiesEntry,
     }
 
-    const entry = queryEntriesRegistry.get(key)!
     // automatically try to refresh the data if it's expired
+    // TODO: move out of ensure entry. This should be called in specific cases
     entry.fetch()
 
-    return entry as UseDataFetchingQueryEntry<TResult, TError>
+    return entry
   }
 
   /**
@@ -117,13 +180,14 @@ export const useDataFetchingStore = defineStore('PiniaColada', () => {
    * @param refresh - whether to force a refresh of the data
    */
   function invalidateEntry(key: UseQueryKey, refresh = false) {
-    if (!queryEntriesRegistry.has(key)) {
+    if (!entryPropertiesRegistry.has(key)) {
+      // TODO: dev only
       console.warn(
         `⚠️ trying to invalidate "${String(key)}" but it's not in the registry`
       )
       return
     }
-    const entry = queryEntriesRegistry.get(key)!
+    const entry = entryPropertiesRegistry.get(key)!
 
     if (entry.previous) {
       // will force a fetch next time
@@ -138,10 +202,23 @@ export const useDataFetchingStore = defineStore('PiniaColada', () => {
     }
   }
 
+  function prefetch(key: UseQueryKey) {
+    const entry = entryPropertiesRegistry.get(key)
+    if (!entry) {
+      console.warn(
+        `⚠️ trying to prefetch "${String(key)}" but it's not in the registry`
+      )
+      return
+    }
+    entry.fetch()
+  }
+
   return {
-    dataRegistry,
-    errorRegistry,
-    isFetchingRegistry,
+    entryStateRegistry,
+
+    // dataRegistry,
+    // errorRegistry,
+    // isFetchingRegistry,
 
     ensureEntry,
     invalidateEntry,

src/use-query.spec.ts

@@ -16,26 +16,28 @@ describe('useQuery', () => {
 
   const runTimers = async (onlyPending = true) => {
     if (onlyPending) {
-      vi.runOnlyPendingTimers()
+      await vi.runOnlyPendingTimersAsync()
     } else {
-      vi.runAllTimers()
+      // vi.runAllTimers()
+      await vi.runAllTimersAsync()
     }
     await nextTick()
   }
 
-  const mountSimple = () =>
-    mount(
+  const mountSimple = () => {
+    const spy = vi.fn(async () => {
+      console.log('fetching')
+      await delay(0)
+      console.log('fetching done')
+      return 42
+    })
+    const wrapper = mount(
       {
         render: () => null,
         setup() {
           return {
             ...useQuery({
-              fetcher: async () => {
-                console.log('fetching')
-                await delay(0)
-                console.log('!fetching')
-                return 42
-              },
+              fetcher: spy,
               key: 'foo',
             }),
           }
@@ -47,12 +49,14 @@ describe('useQuery', () => {
         },
       }
     )
+    return { wrapper }
+  }
 
   it('renders the loading state initially', async () => {
-    const wrapper = mountSimple()
+    const { wrapper } = mountSimple()
 
     expect(wrapper.vm.data).toBeUndefined()
-    expect(wrapper.vm.isPending).toBe(true)
+    // expect(wrapper.vm.isPending).toBe(true)
     expect(wrapper.vm.isFetching).toBe(true)
     expect(wrapper.vm.error).toBeNull()
 
@@ -63,6 +67,6 @@ describe('useQuery', () => {
     expect(wrapper.vm.error).toBeNull()
     // FIXME: this should be false but it's not reactive yet
     // expect(wrapper.vm.isPending).toBe(false)
-    // expect(wrapper.vm.isFetching).toBe(false)
+    expect(wrapper.vm.isFetching).toBe(false)
   })
 })

src/use-query.ts

@@ -6,14 +6,16 @@ import {
   onServerPrefetch,
   toValue,
   onScopeDispose,
+  ShallowRef,
+  Ref,
 } from 'vue'
 import { useDataFetchingStore } from './data-fetching-store'
 
 export interface UseQueryReturn<TResult = unknown, TError = Error> {
-  data: ComputedRef<TResult | undefined>
-  error: ComputedRef<TError | null>
-  isFetching: ComputedRef<boolean>
-  isPending: ComputedRef<boolean>
+  data: Ref<TResult | undefined>
+  error: ShallowRef<TError | null>
+  isFetching: Ref<boolean>
+  isPending: Ref<boolean>
   refresh: () => Promise<void>
 }
 
@@ -72,8 +74,8 @@ export interface UseQueryOptions<TResult = unknown> {
   gcTime?: number
 
   initialData?: () => TResult
-  refetchOnWindowFocus?: boolean
-  refetchOnReconnect?: boolean
+  refetchOnWindowFocus?: boolean | 'force'
+  refetchOnReconnect?: boolean | 'force'
 }
 
 /**
@@ -82,8 +84,9 @@ export interface UseQueryOptions<TResult = unknown> {
 export const USE_QUERY_DEFAULTS = {
   staleTime: 1000 * 5, // 5 seconds
   gcTime: 1000 * 60 * 5, // 5 minutes
-  refetchOnWindowFocus: true as boolean,
-  refetchOnReconnect: true as boolean,
+  // avoid type narrowing to `true`
+  refetchOnWindowFocus: true as UseQueryOptions['refetchOnWindowFocus'],
+  refetchOnReconnect: true as UseQueryOptions['refetchOnReconnect'],
 } satisfies Partial<UseQueryOptions>
 export type UseQueryOptionsWithDefaults<TResult> = typeof USE_QUERY_DEFAULTS &
   UseQueryOptions<TResult>
@@ -140,13 +143,15 @@ export function useQuery<TResult, TError = Error>(
     }
   }
 
+  // TODO: handle if key is reactive
+
   const queryReturn = {
-    // TODO: optimize so we create only one computed per entry. We could have an application plugin that creates an effectScope and allows us to inject the scope to create entries
-    data: computed(() => entry.value.data()),
-    error: computed(() => entry.value.error()),
-    isFetching: computed(() => entry.value.isFetching()),
-    isPending: computed(() => entry.value.isPending()),
+    data: entry.value.data,
+    error: entry.value.error,
+    isFetching: entry.value.isFetching,
+    isPending: entry.value.isPending,
 
+    // TODO: do we need to force bound to the entry?
     refresh: () => entry.value.refresh(),
   } satisfies UseQueryReturn<TResult, TError>