feat: return a promise of data only

BREAKING CHANGE: since data loaders aren't meant to be awaited in script
setup (they are awaited at the navigation level), they now return a
promise of the raw data only, not of the UseDataLoaderReturn, to make it
clearer that this syntax is a bit special and should only be used within
nested loaders. This change also brings other benefits like allowing
lazy loaders to be awaited within loaders without changing their usage
outside, in components. Also, allowing different types of commit while
still allowing data to be awaited within loaders.

src/data-fetching_new/createDataLoader.ts

@@ -6,7 +6,7 @@ import type {
   RouteLocationNormalizedLoaded,
   Router,
 } from 'vue-router'
-import { IS_USE_DATA_LOADER_KEY } from './symbols'
+import { IS_USE_DATA_LOADER_KEY, STAGED_NO_VALUE } from './symbols'
 import { _Awaitable } from '../core/utils'
 import { _PromiseMerged } from './utils'
 
@@ -67,7 +67,7 @@ export interface DataLoaderEntryBase<
    * Data that was staged by a loader. This is used to avoid showing the old data while the new data is loading. Calling
    * the internal `commit()` function will replace the data with the staged data.
    */
-  staged: Data | null
+  staged: Data | typeof STAGED_NO_VALUE
 
   commit(to: RouteLocationNormalizedLoaded): void
 }
@@ -181,7 +181,7 @@ export interface UseDataLoader<
   // maybe a context argument
   // _fn: () => _Awaitable<Data>
 
-  (): _PromiseMerged<UseDataLoaderResult<isLazy, Data>>
+  (): _PromiseMerged<Data, UseDataLoaderResult<isLazy, Data>>
 
   _: UseDataLoaderInternals<isLazy, Data>
 }

src/data-fetching_new/defineLoader.spec.ts

@@ -25,10 +25,8 @@ import {
   dataOneSpy,
   dataTwoSpy,
   useDataOne,
-  useDataTwo,
 } from '~/tests/data-loaders/loaders'
 import type { RouteLocationNormalizedLoaded } from 'vue-router'
-import ComponentWithLoader from '~/tests/data-loaders/ComponentWithLoader.vue'
 
 describe('defineLoader', () => {
   let removeGuards = () => {}
@@ -101,25 +99,66 @@ describe('defineLoader', () => {
   }
 
   describe('Basic defineLoader', () => {
-    it('sets the value during navigation', async () => {
-      const spy = vi
-        .fn<unknown[], Promise<string>>()
-        .mockResolvedValueOnce('resolved')
-      const { wrapper, useData, router } = singleLoaderOneRoute(
-        // TODO: should allow sync loaders too?
-        defineLoader(async () => spy())
-      )
-      expect(spy).not.toHaveBeenCalled()
-      await router.push('/fetch')
-      expect(wrapper.get('#error').text()).toBe('')
-      expect(wrapper.get('#pending').text()).toBe('false')
-      expect(wrapper.get('#data').text()).toBe('resolved')
-      expect(spy).toHaveBeenCalledTimes(1)
-      const { data } = useData()
-      expect(data.value).toEqual('resolved')
-    })
+    describe.each(['immediate', 'after-load'] as const)(
+      'commit: %s',
+      (commit) => {
+        describe.each([true, false] as const)('lazy: %s', (lazy) => {
+          it(`can resolve an "null" with lazy: ${lazy}, commit: ${commit}`, async () => {
+            const spy = vi
+              .fn<unknown[], Promise<unknown>>()
+              .mockResolvedValueOnce(null)
+            const { wrapper, useData, router } = singleLoaderOneRoute(
+              defineLoader(async () => spy(), { lazy, commit })
+            )
+            await router.push('/fetch')
+            expect(spy).toHaveBeenCalledTimes(1)
+            const { data } = useData()
+            expect(data.value).toEqual(null)
+          })
+
+          it(`sets the value after navigation with lazy: ${lazy}, commit: ${commit}`, async () => {
+            const spy = vi
+              .fn<unknown[], Promise<string>>()
+              .mockResolvedValueOnce('resolved')
+            const { wrapper, useData, router } = singleLoaderOneRoute(
+              // TODO: should allow sync loaders too?
+              defineLoader(async () => spy(), { lazy, commit })
+            )
+            expect(spy).not.toHaveBeenCalled()
+            await router.push('/fetch')
+            expect(wrapper.get('#error').text()).toBe('')
+            expect(wrapper.get('#pending').text()).toBe('false')
+            expect(wrapper.get('#data').text()).toBe('resolved')
+            expect(spy).toHaveBeenCalledTimes(1)
+            const { data } = useData()
+            expect(data.value).toEqual('resolved')
+          })
+
+          it(`can be forced refreshed with lazy: ${lazy}, commit: ${commit}`, async () => {
+            const spy = vi
+              .fn<unknown[], Promise<string>>()
+              .mockResolvedValueOnce('resolved 1')
+            const { wrapper, router, useData } = singleLoaderOneRoute(
+              defineLoader(async () => spy())
+            )
+            await router.push('/fetch')
+            expect(spy).toHaveBeenCalledTimes(1)
+            const { data, refresh } = useData()
+            expect(data.value).toEqual('resolved 1')
+            spy.mockResolvedValueOnce('resolved 2')
+            await refresh()
+            expect(spy).toHaveBeenCalledTimes(2)
+            expect(data.value).toEqual('resolved 2')
+            spy.mockResolvedValueOnce('resolved 3')
+            await refresh()
+            expect(spy).toHaveBeenCalledTimes(3)
+            expect(data.value).toEqual('resolved 3')
+          })
+        })
+      }
+    )
 
-    it('blocks navigation by default', async () => {
+    it('blocks navigation by default (non lazy)', async () => {
       const [spy, resolve, reject] = mockPromise('resolved')
       const { useData, router } = singleLoaderOneRoute(defineLoader(spy))
       const p = router.push('/fetch')
@@ -154,7 +193,7 @@ describe('defineLoader', () => {
       expect(wrapper.get('#data').text()).toBe('resolved')
     })
 
-    it('should abort the navigation if the loader throws', async () => {
+    it('should abort the navigation if the non lazy loader throws', async () => {
       const { wrapper, useData, router } = singleLoaderOneRoute(
         defineLoader(async () => {
           throw new Error('nope')
@@ -168,27 +207,6 @@ describe('defineLoader', () => {
       expect(data.value).toEqual(undefined)
     })
 
-    it('can be forced refreshed', async () => {
-      const spy = vi
-        .fn<unknown[], Promise<string>>()
-        .mockResolvedValueOnce('resolved 1')
-      const { wrapper, router, useData } = singleLoaderOneRoute(
-        defineLoader(async () => spy())
-      )
-      await router.push('/fetch')
-      expect(spy).toHaveBeenCalledTimes(1)
-      const { data, refresh } = useData()
-      expect(data.value).toEqual('resolved 1')
-      spy.mockResolvedValueOnce('resolved 2')
-      await refresh()
-      expect(spy).toHaveBeenCalledTimes(2)
-      expect(data.value).toEqual('resolved 2')
-      spy.mockResolvedValueOnce('resolved 3')
-      await refresh()
-      expect(spy).toHaveBeenCalledTimes(3)
-      expect(data.value).toEqual('resolved 3')
-    })
-
     it('discards a pending load if a new navigation happens', async () => {
       let calls = 0
       let resolveFirstCall!: (val?: unknown) => void
@@ -252,13 +270,13 @@ describe('defineLoader', () => {
         .fn<[to: RouteLocationNormalizedLoaded], Promise<unknown>>()
         .mockImplementation(async (to) => {
           rootCalls++
-          const { data } = await useNestedLoader()
+          const data = await useNestedLoader()
           if (rootCalls === 1) {
             await rootP1
           } else {
             await rootP2
           }
-          return `${data.value},${to.query.p}`
+          return `${data},${to.query.p}`
         })
 
       const { wrapper, useData, router, app } = singleLoaderOneRoute(
@@ -363,13 +381,13 @@ describe('defineLoader', () => {
       const { wrapper, useData, router, app } = singleLoaderOneRoute(
         defineLoader(async (to) => {
           rootCalls++
-          const { data } = await useNestedLoader()
+          const data = await useNestedLoader()
           if (rootCalls === 1) {
             await rootP1
           } else {
             await rootP2
           }
-          return `${data.value},${to.query.p}`
+          return `${data},${to.query.p}`
         })
       )
       const firstNavigation = router.push('/fetch?p=one')
@@ -410,12 +428,12 @@ describe('defineLoader', () => {
       await vi.runAllTimersAsync()
       expect(useDataPromise).toBeInstanceOf(Promise)
       resolve()
-      const { data } = await useDataPromise
+      const data = await useDataPromise
       // await router.getPendingNavigation()
       expect(spy).toHaveBeenCalledTimes(1)
-      expect(data.value).toEqual('resolved')
+      expect(data).toEqual('resolved')
       expect(spy).toHaveBeenCalledTimes(1)
-      expect(data.value).toEqual('resolved')
+      expect(data).toEqual('resolved')
     })
 
     it('can nest loaders', async () => {
@@ -427,9 +445,9 @@ describe('defineLoader', () => {
         .mockResolvedValueOnce('two')
       const useLoaderOne = defineLoader(async () => spyOne())
       const useLoaderTwo = defineLoader(async () => {
-        const { data: one } = await useLoaderOne()
+        const one = await useLoaderOne()
         const two = await spyTwo()
-        return `${one.value},${two}`
+        return `${one},${two}`
       })
       const { wrapper, useData, router } = singleLoaderOneRoute(useLoaderTwo)
       await router.push('/fetch')
@@ -492,6 +510,29 @@ describe('defineLoader', () => {
       expect(one.value).toEqual('one')
       expect(two.value).toEqual('two')
     })
+
+    it('awaits for a lazy loader if used as a nested loader', async () => {
+      const l1 = mockedLoader({ lazy: true, key: 'nested' })
+      const { wrapper, app, router, useData } = singleLoaderOneRoute(
+        defineLoader(
+          async (to) => {
+            const data = await l1.loader()
+            return `${data},${to.query.p}`
+          },
+          { key: 'root' }
+        )
+      )
+
+      const p = router.push('/fetch?p=one')
+      await vi.runOnlyPendingTimersAsync()
+
+      const { data } = useData()
+      expect(data.value).toEqual(undefined)
+
+      l1.resolve('ok')
+      await vi.runOnlyPendingTimersAsync()
+      expect(data.value).toEqual('ok,one')
+    })
   })
 })
 

src/data-fetching_new/defineLoader.ts

@@ -16,6 +16,7 @@ import {
   IS_USE_DATA_LOADER_KEY,
   LOADER_ENTRIES_KEY,
   PENDING_LOCATION_KEY,
+  STAGED_NO_VALUE,
 } from './symbols'
 import { getCurrentContext, setCurrentContext, withinScope } from './utils'
 import { Ref, UnwrapRef, ref, shallowRef } from 'vue'
@@ -157,18 +158,18 @@ export function defineLoader<
     if (_entry.pendingTo === to) {
       // console.log('👉 commit', _entry.staged)
       if (process.env.NODE_ENV === 'development') {
-        if (_entry.staged === null) {
+        if (_entry.staged === STAGED_NO_VALUE) {
           console.warn(
             `Loader "${options.key}"'s "commit()" was called but there is no staged data.`
           )
         }
       }
       // if the entry is null, it means the loader never resolved, maybe there was an error
-      if (_entry.staged !== null) {
+      if (_entry.staged !== STAGED_NO_VALUE) {
         // @ts-expect-error: staged starts as null but should always be set at this point
         _entry.data.value = _entry.staged
       }
-      _entry.staged = null
+      _entry.staged = STAGED_NO_VALUE
       _entry.pendingTo = null
 
       // children entries cannot be committed from the navigation guard, so the parent must tell them
@@ -259,7 +260,9 @@ export function defineLoader<
 
     // load ensures there is a pending load
     const promise = entry.pendingLoad!.then(() => {
-      return useDataLoaderResult
+      // nested loaders might wait for all loaders to be ready before setting data
+      // so we need to return the staged value if it exists as it will be the latest one
+      return entry!.staged === STAGED_NO_VALUE ? data.value : entry!.staged
     })
 
     return Object.assign(promise, useDataLoaderResult)
@@ -331,7 +334,7 @@ export function createDefineLoaderEntry<
         isReady: false,
         pendingLoad: null,
         pendingTo: null,
-        staged: null,
+        staged: STAGED_NO_VALUE,
         commit,
       } satisfies DataLoaderEntryBase<isLazy, Data>)
   )

src/data-fetching_new/symbols.ts

@@ -21,3 +21,9 @@ export const IS_USE_DATA_LOADER_KEY = Symbol()
  * @internal
  */
 export const PENDING_LOCATION_KEY = Symbol()
+
+/**
+ * Symbol used to know there is no value staged for the loader and that commit should be skipped.
+ * @internal
+ */
+export const STAGED_NO_VALUE = Symbol()

tests/utils.ts

@@ -1,4 +1,8 @@
 import { vi } from 'vitest'
+import {
+  DefineDataLoaderOptions,
+  defineLoader,
+} from '~/src/data-fetching_new/defineLoader'
 
 export const delay = (ms: number) =>
   new Promise((resolve) => setTimeout(resolve, ms))
@@ -33,3 +37,16 @@ export function mockPromise<Resolved, Err>(resolved: Resolved, rejected?: Err) {
 
   return [spy, resolve, reject] as const
 }
+
+export function mockedLoader(
+  // boolean is easier to handle for router mock
+  options?: DefineDataLoaderOptions<boolean>
+) {
+  const [spy, resolve, reject] = mockPromise('ok', 'ko')
+  return {
+    spy,
+    resolve,
+    reject,
+    loader: defineLoader(async () => await spy(), options),
+  }
+}