fix: discard loads from canceled navigations

Fix #200

src/data-fetching_new/defineLoader.spec.ts

@@ -155,6 +155,45 @@ describe('defineLoader', () => {
             expect(data.value).toEqual('resolved 3')
           })
         })
+
+        it.todo(
+          `should abort the navigation if the non lazy loader throws, commit: ${commit}`,
+          async () => {
+            const { wrapper, useData, router } = singleLoaderOneRoute(
+              defineLoader(
+                async () => {
+                  throw new Error('nope')
+                },
+                { commit }
+              )
+            )
+            await router.push('/fetch')
+            expect(wrapper.get('#error').text()).toBe('Error: nope')
+            expect(wrapper.get('#pending').text()).toBe('false')
+            expect(wrapper.get('#data').text()).toBe('')
+            const { data } = useData()
+            expect(router.currentRoute.value.path).not.toBe('/fetch')
+            expect(data.value).toEqual(undefined)
+          }
+        )
+
+        it(`should not abort the navigation if the lazy loader throws, commit: ${commit}`, async () => {
+          const { wrapper, useData, router } = singleLoaderOneRoute(
+            defineLoader(
+              async () => {
+                throw new Error('nope')
+              },
+              { lazy: true, commit }
+            )
+          )
+          await router.push('/fetch')
+          expect(wrapper.get('#error').text()).toBe('Error: nope')
+          expect(wrapper.get('#pending').text()).toBe('false')
+          expect(wrapper.get('#data').text()).toBe('')
+          expect(router.currentRoute.value.path).toBe('/fetch')
+          const { data } = useData()
+          expect(data.value).toEqual(undefined)
+        })
       }
     )
 
@@ -193,20 +232,6 @@ describe('defineLoader', () => {
       expect(wrapper.get('#data').text()).toBe('resolved')
     })
 
-    it('should abort the navigation if the non lazy loader throws', async () => {
-      const { wrapper, useData, router } = singleLoaderOneRoute(
-        defineLoader(async () => {
-          throw new Error('nope')
-        })
-      )
-      await router.push('/fetch')
-      expect(wrapper.get('#error').text()).toBe('Error: nope')
-      expect(wrapper.get('#pending').text()).toBe('false')
-      expect(wrapper.get('#data').text()).toBe('')
-      const { data } = useData()
-      expect(data.value).toEqual(undefined)
-    })
-
     it('discards a pending load if a new navigation happens', async () => {
       let calls = 0
       let resolveFirstCall!: (val?: unknown) => void
@@ -415,6 +440,65 @@ describe('defineLoader', () => {
       // expect(nestedCalls).toEqual(2)
     })
 
+    it('discards a pending load if trying to navigate back to the current location', async () => {
+      let calls = 0
+      let resolveCall1!: (val?: unknown) => void
+      let resolveCall2!: (val?: unknown) => void
+      let resolveCall3!: (val?: unknown) => void
+      const p1 = new Promise((r) => (resolveCall1 = r))
+      const p2 = new Promise((r) => (resolveCall2 = r))
+      const p3 = new Promise((r) => (resolveCall3 = r))
+      const spy = vi
+        .fn<[to: RouteLocationNormalizedLoaded], Promise<string>>()
+        .mockImplementation(async (to) => {
+          calls++
+          // the first one should be skipped
+          if (calls === 2) {
+            await p1
+          } else if (calls === 3) {
+            await p2
+          } else if (calls === 4) {
+            await p3
+            // this should never happen or be used because the last navigation is considered duplicated
+            return 'ko'
+          }
+          return to.query.p as string
+        })
+      const { wrapper, useData, router } = singleLoaderOneRoute(
+        defineLoader(spy)
+      )
+      // set the initial location
+      await router.push('/fetch?p=ok')
+
+      const { data } = useData()
+      expect(spy).toHaveBeenCalledTimes(1)
+      expect(data.value).toEqual('ok')
+
+      // try running two navigations to a different location
+      router.push('/fetch?p=ko')
+      await vi.runAllTimersAsync()
+      expect(spy).toHaveBeenCalledTimes(2)
+      router.push('/fetch?p=ko')
+      await vi.runAllTimersAsync()
+      expect(spy).toHaveBeenCalledTimes(3)
+
+      // but roll back to the initial one
+      router.push('/fetch?p=ok')
+      await vi.runAllTimersAsync()
+      // it runs 3 times because in vue router, going from /fetch?p=ok to /fetch?p=ok fails right away, so the loader are never called
+      // We simply don't test it because it doesn't matter, what matters is what value is preserved at the end
+      // expect(spy).toHaveBeenCalledTimes(3)
+
+      resolveCall1()
+      resolveCall2()
+      resolveCall3()
+      await vi.runAllTimersAsync()
+      await router.getPendingNavigation()
+
+      // it preserves the initial value
+      expect(data.value).toEqual('ok')
+    })
+
     it('loader result can be awaited for the data to be ready', async () => {
       const [spy, resolve, reject] = mockPromise('resolved')
 

src/data-fetching_new/defineLoader.ts

@@ -119,6 +119,8 @@ export function defineLoader<
         if (entry.pendingLoad === currentLoad) {
           error.value = e
         }
+        // propagate error
+        // return Promise.reject(e)
       })
       .finally(() => {
         setCurrentContext(currentContext)
@@ -242,7 +244,7 @@ export function defineLoader<
       if (parentEntry === entry) {
         console.warn(`👶❌ "${options.key}" has itself as parent`)
       }
-      console.log(`👶 "${options.key}" has parent ${parentEntry}`)
+      // console.log(`👶 "${options.key}" has parent ${parentEntry}`)
       parentEntry.children.add(entry!)
     }
 

src/data-fetching_new/navigation-guard.ts

@@ -5,7 +5,7 @@ import {
   PENDING_LOCATION_KEY,
 } from './symbols'
 import { IS_CLIENT, isDataLoader, setCurrentContext } from './utils'
-import { UseDataLoader } from './createDataLoader'
+import { isNavigationFailure } from 'vue-router'
 
 /**
  * Setups the different Navigation Guards to collect the data loaders from the route records and then to execute them.
@@ -29,6 +29,8 @@ export function setupRouter(router: Router) {
 
   // guard to add the loaders to the meta property
   const removeLoaderGuard = router.beforeEach((to) => {
+    // global pending location, used by nested loaders to know if they should load or not
+    router[PENDING_LOCATION_KEY] = to
     // Differently from records, this one is reset on each navigation
     // so it must be built each time
     to.meta[LOADER_SET_KEY] = new Set()
@@ -68,6 +70,8 @@ export function setupRouter(router: Router) {
       }
     }
 
+    // TODO: not use record to store loaders, or maybe cleanup here
+
     return Promise.all(lazyLoadingPromises).then(() => {
       for (const record of to.matched) {
         // merge the whole set of loaders
@@ -92,9 +96,6 @@ export function setupRouter(router: Router) {
      * - Collect NavigationResults and call `selectNavigationResult` to select the one to use
      */
 
-    // global pending location, used by nested loaders to know if they should load or not
-    router[PENDING_LOCATION_KEY] = to
-
     // unset the context so all loaders are executed as root loaders
     setCurrentContext([])
     return Promise.all(
@@ -135,6 +136,22 @@ export function setupRouter(router: Router) {
     // TODO: handle errors and navigation failures?
   })
 
+  // listen to duplicated navigation failures to reset the pendingTo and pendingLoad
+  // since they won't trigger the beforeEach or beforeResolve defined above
+  router.afterEach((_to, _from, failure) => {
+    if (
+      isNavigationFailure(failure, 16 /* NavigationFailureType.duplicated */)
+    ) {
+      if (router[PENDING_LOCATION_KEY]) {
+        router[PENDING_LOCATION_KEY].meta[LOADER_SET_KEY]!.forEach((loader) => {
+          loader._.entry.pendingTo = null
+          loader._.entry.pendingLoad = null
+        })
+        router[PENDING_LOCATION_KEY] = null
+      }
+    }
+  })
+
   return () => {
     // @ts-expect-error: must be there in practice
     delete router[LOADER_ENTRIES_KEY]