Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement force option for refreshing and triggeredBy for getCachedData #25850

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 30 additions & 15 deletions packages/nuxt/src/app/composables/asyncData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export type KeyOfRes<Transform extends _Transform> = KeysOf<ReturnType<Transform

export type MultiWatchSources = (WatchSource<unknown> | object)[]

export type GetCachedDataTriggeredBy = 'initial' | 'refresh:hook' | 'refresh:manual' | 'watch'
export interface AsyncDataOptions<
ResT,
DataT = ResT,
Expand All @@ -60,8 +61,9 @@ export interface AsyncDataOptions<
* Provide a function which returns cached data.
* A `null` or `undefined` return value will trigger a fetch.
* Default is `key => nuxt.isHydrating ? nuxt.payload.data[key] : nuxt.static.data[key]` which only caches data when payloadExtraction is enabled.
* triggeredBy is a string that indicates in which case the cached data was requested.
*/
getCachedData?: (key: string) => DataT
getCachedData?: (key: string, triggeredBy?: GetCachedDataTriggeredBy) => DataT
Copy link
Member

@danielroe danielroe Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in honesty I'm not yet sure about this as an API. But I also might be missing something.

What is the situation where passing force wouldn't be good enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a scenario where you can't set force because watch is triggered - #24332 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on the force option for refresh, could we just add the force option and basically ignore hasCachedData when forced? And always respect hasCachedData when calling refresh from watch?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update the PR πŸ‘

/**
* A function that can be used to alter handler function result after resolving
*/
Expand Down Expand Up @@ -91,7 +93,7 @@ export interface AsyncDataOptions<
}

export interface AsyncDataExecuteOptions {
_initial?: boolean
_triggeredBy?: GetCachedDataTriggeredBy
// TODO: remove boolean option in Nuxt 4
/**
* Force a refresh, even if there is already a pending request. Previous requests will
Expand All @@ -102,6 +104,11 @@ export interface AsyncDataExecuteOptions {
* Boolean values will be removed in a future release.
*/
dedupe?: boolean | 'cancel' | 'defer'
/**
* Do not use potentially cached data from getCachedData and perform a new request
* @default false
*/
force?: boolean
}

export interface _AsyncData<DataT, ErrorT> {
Expand Down Expand Up @@ -239,7 +246,7 @@ export function useAsyncData<
console.warn('[nuxt] `boolean` values are deprecated for the `dedupe` option of `useAsyncData` and will be removed in the future. Use \'cancel\' or \'defer\' instead.')
}

const hasCachedData = () => ![null, undefined].includes(options.getCachedData!(key) as any)
const hasCachedData = (triggeredBy?: GetCachedDataTriggeredBy) => ![null, undefined].includes(options.getCachedData!(key, triggeredBy) as any)

// Create or use a shared asyncData entity
if (!nuxtApp._asyncData[key] || !options.immediate) {
Expand All @@ -248,8 +255,8 @@ export function useAsyncData<
const _ref = options.deep ? ref : shallowRef

nuxtApp._asyncData[key] = {
data: _ref(options.getCachedData!(key) ?? options.default!()),
pending: ref(!hasCachedData()),
data: _ref(options.getCachedData!(key, 'initial') ?? options.default!()),
pending: ref(!hasCachedData('initial')),
error: toRef(nuxtApp.payload._errors, key),
status: ref('idle')
}
Expand All @@ -258,7 +265,7 @@ export function useAsyncData<
// TODO: Else, somehow check for conflicting keys with different defaults or fetcher
const asyncData = { ...nuxtApp._asyncData[key] } as AsyncData<DataT | DefaultT, (NuxtErrorDataT extends Error | NuxtError ? NuxtErrorDataT : NuxtError<NuxtErrorDataT>)>

asyncData.refresh = asyncData.execute = (opts = {}) => {
asyncData.refresh = asyncData.execute = (opts = { _triggeredBy: 'refresh:manual' }) => {
if (nuxtApp._asyncDataPromises[key]) {
if (isDefer(opts.dedupe ?? options.dedupe)) {
// Avoid fetching same key more than once at a time
Expand All @@ -267,9 +274,17 @@ export function useAsyncData<
(nuxtApp._asyncDataPromises[key] as any).cancelled = true
}
// Avoid fetching same key that is already fetched
if ((opts._initial || (nuxtApp.isHydrating && opts._initial !== false)) && hasCachedData()) {
return Promise.resolve(options.getCachedData!(key))
if(opts._triggeredBy === 'initial' && !opts.force && nuxtApp.isHydrating && hasCachedData(opts._triggeredBy)) {
return Promise.resolve(options.getCachedData!(key, opts._triggeredBy))
}

// If cache changed based on the triggeredBy, set new data result
if (!opts.force && hasCachedData(opts._triggeredBy)) {
const result = options.getCachedData!(key, opts._triggeredBy)
asyncData.data.value = result
return Promise.resolve(result)
}

asyncData.pending.value = true
asyncData.status.value = 'pending'
// TODO: Cancel previous promise
Expand Down Expand Up @@ -318,7 +333,7 @@ export function useAsyncData<
return nuxtApp._asyncDataPromises[key]!
}

const initialFetch = () => asyncData.refresh({ _initial: true })
const initialFetch = () => asyncData.refresh({ _triggeredBy: 'initial' })

const fetchOnServer = options.server !== false && nuxtApp.payload.serverRendered

Expand Down Expand Up @@ -352,7 +367,7 @@ export function useAsyncData<
}
}

if (fetchOnServer && nuxtApp.isHydrating && (asyncData.error.value || hasCachedData())) {
if (fetchOnServer && nuxtApp.isHydrating && (asyncData.error.value || hasCachedData('initial'))) {
// 1. Hydration (server: true): no fetch
asyncData.pending.value = false
asyncData.status.value = asyncData.error.value ? 'error' : 'success'
Expand All @@ -365,11 +380,11 @@ export function useAsyncData<
initialFetch()
}
if (options.watch) {
watch(options.watch, () => asyncData.refresh())
watch(options.watch, () => asyncData.refresh({ _triggeredBy: 'watch' }))
}
const off = nuxtApp.hook('app:data:refresh', async (keys) => {
const off = nuxtApp.hook('app:data:refresh', async (keys, force) => {
if (!keys || keys.includes(key)) {
await asyncData.refresh()
await asyncData.refresh({ force, _triggeredBy: 'refresh:hook' })
}
})
if (instance) {
Expand Down Expand Up @@ -462,15 +477,15 @@ export function useNuxtData<DataT = any> (key: string): { data: Ref<DataT | null
}

/** @since 3.0.0 */
export async function refreshNuxtData (keys?: string | string[]): Promise<void> {
export async function refreshNuxtData (keys?: string | string[], force?: boolean): Promise<void> {
if (import.meta.server) {
return Promise.resolve()
}

await new Promise<void>(resolve => onNuxtReady(resolve))

const _keys = keys ? toArray(keys) : undefined
await useNuxtApp().hooks.callHookParallel('app:data:refresh', _keys)
await useNuxtApp().hooks.callHookParallel('app:data:refresh', _keys, force)
}

/** @since 3.0.0 */
Expand Down
2 changes: 1 addition & 1 deletion packages/nuxt/src/app/nuxt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export interface RuntimeNuxtHooks {
'app:error': (err: any) => HookResult
'app:error:cleared': (options: { redirect?: string }) => HookResult
'app:chunkError': (options: { error: any }) => HookResult
'app:data:refresh': (keys?: string[]) => HookResult
'app:data:refresh': (keys?: string[], force?: boolean) => HookResult
'app:manifest:update': (meta?: NuxtAppManifestMeta) => HookResult
'link:prefetch': (link: string) => HookResult
'page:start': (Component?: VNode) => HookResult
Expand Down
15 changes: 11 additions & 4 deletions playground/app.vue
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
<script setup lang="ts">
const { data, refresh } = await useAsyncData('key', () => Promise.resolve('something'), {
getCachedData: (_, triggeredBy) => {
if(triggeredBy === 'refresh:manual') {
return 1
}
return 0
}
})
</script>

<template>
<!-- Edit this file to play around with Nuxt but never commit changes! -->
<div>
Nuxt 3 Playground
{{ data }}
<button @click="refresh()" />
</div>
</template>

<style scoped>

</style>
<style scoped></style>
43 changes: 43 additions & 0 deletions test/nuxt/composables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,13 @@ describe('useAsyncData', () => {
expect(data.data.value).toMatchInlineSnapshot('"test"')
})

it('should be refreshable with force and cache', async () => {
await useAsyncData('key', () => Promise.resolve('test'), { getCachedData: () => 'cached' })
await refreshNuxtData('key', { force: true })
const data = useNuxtData('key')
expect(data.data.value).toMatchInlineSnapshot('"test"')
})

it('allows custom access to a cache', async () => {
const { data } = await useAsyncData(() => ({ val: true }), { getCachedData: () => ({ val: false }) })
expect(data.value).toMatchInlineSnapshot(`
Expand All @@ -231,6 +238,42 @@ describe('useAsyncData', () => {
`)
})

it('will use cache on refresh by default', async () => {
const { data, refresh } = await useAsyncData(() => 'other value', { getCachedData: () => 'cached' })
expect(data.value).toBe('cached')
await refresh()
expect(data.value).toBe('cached')
})

it('will not use cache with force option', async () => {
let called = 0
const fn = () => called++
const { data, refresh } = await useAsyncData(() => 'other value', { getCachedData: () => fn() })
await refresh({ force: true })
expect(data.value).toBe('other value')
})

it('getCachedData should receive triggeredBy on initial fetch', async () => {
const { data } = await useAsyncData(() => '', { getCachedData: (_, triggeredBy) => triggeredBy })
expect(data.value).toBe('initial')
})

it('getCachedData should receive triggeredBy on manual refresh', async () => {
const { data, refresh } = await useAsyncData(() => '', {
getCachedData: (_, triggeredBy) => triggeredBy
})
await refresh()
expect(data.value).toBe('refresh:manual')
})

it('getCachedData should receive triggeredBy on watch', async () => {
const number = ref(0)
const { data } = await useAsyncData(() => '', { getCachedData: (_, triggeredBy) => triggeredBy, watch: [number] })
number.value = 1
await new Promise(resolve => setTimeout(resolve, 1))
expect(data.value).toBe('watch')
})

it('should use default while pending', async () => {
const promise = useAsyncData(() => Promise.resolve('test'), { default: () => 'default' })
const { data, pending } = promise
Expand Down