diff --git a/src/data-fetching_new/defineLoader.spec.ts b/src/data-fetching_new/defineLoader.spec.ts index a85e5adf..2e673dc4 100644 --- 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 index 25faf349..0723e00d 100644 --- 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 index f66cdc7d..73c9a736 100644 --- 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