Skip to content

Commit

Permalink
fix: trigger loaders only once when nested
Browse files Browse the repository at this point in the history
  • Loading branch information
posva committed Aug 3, 2022
1 parent 1b7e6b3 commit 4a13819
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 9 deletions.
24 changes: 24 additions & 0 deletions src/data-fetching/defineLoader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,30 @@ describe('defineLoader', () => {
expect(user.value).toEqual({ name: 'edu' })
})

it('only calls reused loaders once', async () => {
const spy = vi
.fn<any[], Promise<{ user: { name: string } }>>()
.mockResolvedValue({ user: { name: 'edu' } })
const useOne = defineLoader(spy)
const useLoader = defineLoader(async () => {
const { user } = await useOne()
return { user, local: user.value.name }
})

let p = useLoader._.load(route, router)
await useOne._.load(route, router)
await p
expect(spy).toHaveBeenCalledTimes(1)

useOne().invalidate()
useLoader().invalidate()

// the reverse order
await useOne._.load(route, router)
p = useLoader._.load(route, router)
expect(spy).toHaveBeenCalledTimes(2)
})

it('invalidated nested loaders invalidate a loader (by cache)', async () => {
const spy = vi
.fn<any[], Promise<{ user: { name: string } }>>()
Expand Down
18 changes: 9 additions & 9 deletions src/data-fetching/defineLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,6 @@ export function defineLoader<P extends Promise<any>, isLazy extends boolean>(
const router = _router || useRouter()
const route = _route || useRoute()

// this ensures the entry exists
// if (!options.lazy && !parentEntry) {
// throw new Error('noooooo')
// }

if (
// no cache: we need to load
!cache.has(router) ||
Expand Down Expand Up @@ -212,7 +207,10 @@ export function defineLoader<P extends Promise<any>, isLazy extends boolean>(
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) {
setCurrentContext([entry, router, trackedRoute])
// we could use trackedRoute here but that would break the comparison between currentRoute and route
// we could also just add a private property to check if it's the same navigation, but we actually need
// each loader to have its own tracked route and check nested loaders tracked properties within each loader
setCurrentContext([entry, router, route])
}
const thisPromise = (pendingPromise = loader(trackedRoute)
.then((data) => {
Expand Down Expand Up @@ -264,14 +262,16 @@ export function defineLoader<P extends Promise<any>, isLazy extends boolean>(
function shouldFetchAgain(
entry: DataLoaderCacheEntry<any>,
route: RouteLocationNormalizedLoaded
) {
// TODO: check entry.loaders
): boolean {
return (
// manually invalidated
!entry.when ||
!includesParams(route.params, entry.params) ||
!includesParams(route.query, entry.query) ||
(entry.hash != null && entry.hash !== route.hash)
(entry.hash != null && entry.hash !== route.hash) ||
Array.from(entry.loaders).some((childEntry) =>
shouldFetchAgain(childEntry, route)
)
)
}

Expand Down

0 comments on commit 4a13819

Please sign in to comment.