diff --git a/src/data-fetching/dataCache.ts b/src/data-fetching/dataCache.ts index a8571fca3..b1565349e 100644 --- a/src/data-fetching/dataCache.ts +++ b/src/data-fetching/dataCache.ts @@ -18,7 +18,6 @@ export interface _DataLoaderCacheEntryBase { query: Partial hash: string | null loaders: Set - // TODO: hash /** * Whether there is an ongoing request. diff --git a/src/data-fetching/dataFetchingGuard.ts b/src/data-fetching/dataFetchingGuard.ts index efbf4123a..fabcd2f76 100644 --- a/src/data-fetching/dataFetchingGuard.ts +++ b/src/data-fetching/dataFetchingGuard.ts @@ -31,7 +31,7 @@ export function setupDataFetchingGuard(router: Router) { // TODO: dev only if (added) { console.warn( - '[vue-router]: Data fetching guard added twice. Make sure to remove the extra call' + '[vue-router]: Data fetching guard added twice. Make sure to remove the extra call.' ) return } diff --git a/src/data-fetching/defineLoader.spec.ts b/src/data-fetching/defineLoader.spec.ts index 54a7a620b..9636f2c16 100644 --- a/src/data-fetching/defineLoader.spec.ts +++ b/src/data-fetching/defineLoader.spec.ts @@ -290,6 +290,33 @@ describe('defineLoader', () => { expect(user.value).toEqual({ name: 'bob' }) expect(userFromOne.value).toEqual({ name: 'bob' }) }) + + it('tracks cached nested loaders', async () => { + const spy = vi + .fn< + [RouteLocationNormalizedLoaded], + Promise<{ user: { name: string } }> + >() + .mockImplementation(async (route) => ({ + user: { name: route.params.id as string }, + })) + const useOne = defineLoader(spy) + const useLoader = defineLoader(async () => { + const { user } = await useOne() + return { local: user.value.name } + }) + + // start the useOne first + let pOne = useOne._.load(route, router) + let p = useLoader._.load(route, router) + await p + expect(spy).toHaveBeenCalledTimes(1) + + useOne().invalidate() + + p = useLoader._.load(route, router) + expect(spy).toHaveBeenCalledTimes(2) + }) }) it('can be refreshed and awaited', async () => { diff --git a/src/data-fetching/defineLoader.ts b/src/data-fetching/defineLoader.ts index 7735a5b0f..5c008e09d 100644 --- a/src/data-fetching/defineLoader.ts +++ b/src/data-fetching/defineLoader.ts @@ -86,12 +86,13 @@ export function defineLoader

, isLazy extends boolean>( if ( // no cache: we need to load !cache.has(router) || - // invoked by the parent, we should load again + // invoked by the parent, we should try to load again parentEntry ) { load(route, router, parentEntry) } + // after calling load, we always have an entry const entry = cache.get(router)! // TODO: reload the page and figure out a way of detecting it @@ -113,12 +114,20 @@ export function defineLoader

, isLazy extends boolean>( // } // } - const promise = Promise.resolve(pendingPromise).then(() => { - // we need to get the data property once again because it has been updated - const { data } = entry - return Object.assign(commonData, isRef(data) ? { data } : data) - }) - // FIXME: finally to set the currentContext + const promise = Promise.resolve(pendingPromise) + .then(() => { + // we need to get the data property once again because it has been updated + const { data } = entry + return Object.assign(commonData, isRef(data) ? { data } : data) + }) + .finally(() => { + // loader still needs to load again if this was a nested loader, we need to tell the parent they depend on us + if (parentEntry) { + parentEntry.loaders.add(entry) + } + // set the correct context for other nested loaders + setCurrentContext(parentEntry && [parentEntry, router, route]) + }) // entry exists because it's created synchronously in `load()` const { data, pending, error } = entry @@ -144,7 +153,11 @@ export function defineLoader

, isLazy extends boolean>( } const dataLoaderResult = Object.assign( commonData, - isRef(data) ? { data } : data + isRef(data) + ? { data } + : // TODO: in dev we could replace this with a proxy if the data === PLACEHOLDER + // to tell the user they are calling the loader without exporting it OR a nested loader without awaiting + data ) return Object.assign(promise, dataLoaderResult) @@ -204,7 +217,6 @@ export function defineLoader

, isLazy extends boolean>( // remember what was the last navigation we fetched this with currentNavigation = route - // TODO: ensure others useUserData() (loaders) can be called with a similar approach as pinia const [trackedRoute, params, query, hash] = trackRoute(route) // if there isn't a pending promise, we set the current context so nested loaders can use it if (!pendingPromise) { @@ -233,16 +245,8 @@ export function defineLoader

, isLazy extends boolean>( entry.pending.value = false } - // FIXME: this section should be moved so it **always** gets appended to a pending promise I need to find a - // failing test first: call the nested loader first, then the parent loader and use another nested loader - // after the one that was already loaded. Then invalidate the second nested loader and see if the parent - // loader still needs to load again if this was a nested loader, we need to tell the parent they depend on us - if (parent) { - parent.loaders.add(entry) - } - - // this part we keep it here because the parent load should unset the context - // we restore the previous context + // NOTE: unfortunately we need to duplicate this part here and on the `finally()` above + // to handle all setCurrentContext(parent && [parent, router, route]) })) } @@ -323,11 +327,11 @@ export function isDataLoader(loader: any): loader is DataLoader { return loader && loader[IsLoader] } -type PromiseMerged = T & Promise +type _PromiseMerged = T & Promise export interface DataLoader { (): true extends isLazy - ? PromiseMerged<_DataLoaderResultLazy> - : PromiseMerged<_DataLoaderResult & ToRefs> + ? _PromiseMerged<_DataLoaderResultLazy> + : _PromiseMerged<_DataLoaderResult & ToRefs> [IsLoader]: true @@ -399,7 +403,6 @@ function trackRoute(route: RouteLocationNormalizedLoaded) { const [params, paramReads] = trackObjectReads(route.params) const [query, queryReads] = trackObjectReads(route.query) let hash: { v: string | null } = { v: null } - // TODO: track `hash` return [ { ...route,