Skip to content

Commit

Permalink
fix: avoid uncatchable rejection
Browse files Browse the repository at this point in the history
  • Loading branch information
posva committed Feb 8, 2024
1 parent 3d341ae commit fa0c794
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 10 deletions.
23 changes: 23 additions & 0 deletions src/data-fetching_new/defineColadaLoader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,29 @@ describe(
app,
}
}

it.todo('avoids refetching fresh data when navigating', async () => {
const query = vi.fn().mockResolvedValue('data')
const useData = defineColadaLoader({
query,
key: (to) => [to.query.q as string],
})

const { router } = singleLoaderOneRoute(useData)

// same key
await router.push('/fetch?q=1&v=1')
expect(query).toHaveBeenCalledTimes(1)
await router.push('/fetch?q=1&v=2')
expect(query).toHaveBeenCalledTimes(1)

// different key
await router.push('/fetch?q=2&v=3')
expect(query).toHaveBeenCalledTimes(2)
// already fetched
await router.push('/fetch?q=1&v=4')
expect(query).toHaveBeenCalledTimes(2)
})
},
// fail faster on unresolved promises
{ timeout: 100 }
Expand Down
16 changes: 11 additions & 5 deletions src/data-fetching_new/defineColadaLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,17 @@ export function defineColadaLoader<Data, isLazy extends boolean>(
} satisfies UseDataLoaderResult

// load ensures there is a pending load
const promise = entry.pendingLoad!.then(() => {
// nested loaders might wait for all loaders to be ready before setting data
// so we need to return the staged value if it exists as it will be the latest one
return entry!.staged === STAGED_NO_VALUE ? ext!.data.value : entry!.staged
})
const promise = entry
.pendingLoad!.then(() => {
// nested loaders might wait for all loaders to be ready before setting data
// so we need to return the staged value if it exists as it will be the latest one
return entry!.staged === STAGED_NO_VALUE
? ext!.data.value
: entry!.staged
})
// we only want the error if we are nesting the loader
// otherwise this will end up in "Unhandled promise rejection"
.catch((e) => (parentEntry ? Promise.reject(e) : null))

return Object.assign(promise, useDataLoaderResult)
}
Expand Down
14 changes: 9 additions & 5 deletions src/data-fetching_new/defineLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,15 @@ export function defineBasicLoader<Data, isLazy extends boolean>(
} satisfies UseDataLoaderResult

// load ensures there is a pending load
const promise = entry.pendingLoad!.then(() => {
// nested loaders might wait for all loaders to be ready before setting data
// so we need to return the staged value if it exists as it will be the latest one
return entry!.staged === STAGED_NO_VALUE ? data.value : entry!.staged
})
const promise = entry
.pendingLoad!.then(() => {
// nested loaders might wait for all loaders to be ready before setting data
// so we need to return the staged value if it exists as it will be the latest one
return entry!.staged === STAGED_NO_VALUE ? data.value : entry!.staged
})
// we only want the error if we are nesting the loader
// otherwise this will end up in "Unhandled promise rejection"
.catch((e) => (parentEntry ? Promise.reject(e) : null))

return Object.assign(promise, useDataLoaderResult)
}
Expand Down
23 changes: 23 additions & 0 deletions tests/data-loaders/tester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,29 @@ export function testDefineLoader<Context = void>(
expect(data.value).toBe(undefined)
expect(router.currentRoute.value.path).toBe('/fetch')
})

it('propagates errors from nested loaders', async () => {
const l1 = mockedLoader({
key: 'nested',
commit,
lazy: false,
})
const { wrapper, app, router, useData } = singleLoaderOneRoute(
loaderFactory({
fn: async (to) => {
const data = await l1.loader()
return `${data},${to.query.p}`
},
key: 'root',
})
)

const p = router.push('/fetch?p=one')
await vi.runOnlyPendingTimersAsync()

l1.reject(new Error('nope'))
await expect(p).rejects.toThrow('nope')
})
}
)

Expand Down

0 comments on commit fa0c794

Please sign in to comment.