diff --git a/src/data-fetching_new/defineLoader.spec.ts b/src/data-fetching_new/defineLoader.spec.ts index 0a4b574f2..2ab5f029c 100644 --- a/src/data-fetching_new/defineLoader.spec.ts +++ b/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>() + .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') diff --git a/src/data-fetching_new/defineLoader.ts b/src/data-fetching_new/defineLoader.ts index e2c13af38..b1310f23a 100644 --- a/src/data-fetching_new/defineLoader.ts +++ b/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!) } diff --git a/src/data-fetching_new/navigation-guard.ts b/src/data-fetching_new/navigation-guard.ts index e4f368ca4..443d97157 100644 --- a/src/data-fetching_new/navigation-guard.ts +++ b/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]