feat: handle thrown errors

posva · Dec 19, 2023 · 2e38544 · 2e38544
1 parent 365e6e6
commit 2e38544
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 41 deletions.
diff --git a/src/data-fetching_new/defineLoader.spec.ts b/src/data-fetching_new/defineLoader.spec.ts
@@ -156,28 +156,23 @@ describe('defineLoader', () => {
           })
         })
 
-        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 }
-              )
+        it(`should abort the navigation if a non lazy loader throws, commit: ${commit}`, async () => {
+          const { wrapper, 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)
-          }
-        )
+          )
+          const onError = vi.fn()
+          router.onError(onError)
+          await expect(router.push('/fetch')).rejects.toThrow('nope')
+          expect(onError).toHaveBeenCalledTimes(1)
+          expect(router.currentRoute.value.path).not.toBe('/fetch')
+        })
 
-        it(`should not abort the navigation if the lazy loader throws, commit: ${commit}`, async () => {
+        it(`should not abort the navigation if a lazy loader throws, commit: ${commit}`, async () => {
           const { wrapper, useData, router } = singleLoaderOneRoute(
             defineLoader(
               async () => {

diff --git a/src/data-fetching_new/defineLoader.ts b/src/data-fetching_new/defineLoader.ts
@@ -18,6 +18,7 @@ import {
   STAGED_NO_VALUE,
 } from './symbols'
 import {
+  IS_CLIENT,
   assign,
   getCurrentContext,
   setCurrentContext,
@@ -122,9 +123,11 @@ export function defineLoader<
         // )
         if (entry.pendingLoad === currentLoad) {
           error.value = e
+          // propagate error if non lazy or during SSR
+          if (!options.lazy || !IS_CLIENT) {
+            return Promise.reject(e)
+          }
         }
-        // propagate error
-        // return Promise.reject(e)
       })
       .finally(() => {
         setCurrentContext(currentContext)
@@ -155,16 +158,7 @@ export function defineLoader<
     this: DataLoaderEntryBase,
     to: RouteLocationNormalizedLoaded
   ) {
-    if (!this) {
-      if (process.env.NODE_ENV === 'development') {
-        throw new Error(
-          `Loader "${options.key}"'s "commit()" was called before it was loaded once. This will fail in production.`
-        )
-      }
-      return
-    }
-
-    if (this.pendingTo === to) {
+    if (this.pendingTo === to && !this.error.value) {
       // console.log('👉 commit', this.staged)
       if (process.env.NODE_ENV === 'development') {
         if (this.staged === STAGED_NO_VALUE) {

diff --git a/src/data-fetching_new/navigation-guard.ts b/src/data-fetching_new/navigation-guard.ts
@@ -87,10 +87,6 @@ export function setupLoaderGuard(router: Router) {
 
   const removeDataLoaderGuard = router.beforeResolve((to) => {
     const loaders = Array.from(to.meta[LOADER_SET_KEY] || [])
-    // console.log(
-    //   'Invoking data loaders',
-    //   Array.from(to.meta[LOADER_SET_KEY]?.values() || []).map((fn) => fn.name)
-    // )
     /**
      * - ~~Map the loaders to an array of promises~~
      * - ~~Await all the promises (parallel)~~
@@ -106,11 +102,10 @@ export function setupLoaderGuard(router: Router) {
         if (!server && !IS_CLIENT) {
           return
         }
+        // keep track of loaders that should be committed after all loaders are done
         const ret = loader._.load(to, router).then(() => {
-          if (lazy || commit === 'immediate') {
-            // TODO: refactor, it should be done here, it's better
-            // loader._.entry.commit(to)
-          } else if (commit === 'after-load') {
+          // for immediate loaders, the load function handles this
+          if (commit === 'after-load') {
             return loader
           }
         })
@@ -134,7 +129,8 @@ export function setupLoaderGuard(router: Router) {
         // NOTE: could this be dev only?
         // isFetched = true
       })
-    // TODO: handle errors and navigation failures?
+    // no catch so errors are propagated to the router
+    // TODO: handle navigation failures?
   })
 
   // listen to duplicated navigation failures to reset the pendingTo and pendingLoad