fix(nuxt): use single synced asyncdata instance per key#31373
Conversation
Co-authored-by: Alexander Lichter <github@lichter.io>
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #31373 will not alter performanceComparing Summary
|
Co-authored-by: Damian Głowala <damian.glowala.rebkow@gmail.com>
|
After this PR, would From my testing, when the Here is the test code I used: <script lang="ts">
const fetch = () => {
return new Promise((resolve) => {
console.log('fetch...')
setTimeout(() => {
resolve('Nuxt Data 1')
}, 1000)
})
}
function useMyAsyncData() {
return useAsyncData('NUXT_DATA', fetch, {
dedupe: 'cancel' // or 'defer'
})
}
</script>
<script setup lang="ts">
const { data: d1 } = useMyAsyncData()
const { data: d2 } = useMyAsyncData()
</script>Would it make sense to default |
WalkthroughThe changes introduce a new experimental configuration option, granularCachedData, which controls whether cached responses from asynchronous data fetching are utilised during refreshes. The function signatures for both 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@danielroe I'm pulling in the
Exampleconst fetchKey = computed((): string => `page-${route.path.replace(/\//g, '-')}`)
const { data } = await useFetch('/api/v3/pages/my-page', {
key: fetchKey,
}) |
|
@adamdehaven this is expected (from a Nuxt point of view) as the fetch instance is global and never unmounted/cleaned up. You can wrap it in a composable. Or call |
I actually tried calling |
|
@danielroe Shouldn't this feature also apply to |
|
@danielroe I believe there is a breaking change regarding FormData handling in fetch.js due to the newly added statement: It originates from ohash. That library cannot hash File objects. if (opts.body) {
segments.push(hash(toValue(opts.body)))
}When I attempt to upload a FormData containing a File in the body, I encounter the following error: const formData = new FormData();
formData.append("image", new_avatar.value);
await useAPI('/company', {
method: 'POST',
body: formData,
})My current solution is to downgrade v3.16.2 |
|
There seems to be a breaking change in there as well, on route change without remounting components (via I'm trying to dig what's causing this Okay so after digging, this is entirely due to the key now being computed based on the params This means that if any of the params change the key changes and it switches to another data. But that means there is no data until it's fetched again while before what it would do is keep the previous data but fetch the new one while the previous data was available, this is a problem when you are expecting to always have data via top level A workaround for now seems to be to hardcode a static key |
|
@danielroe Thank you so much for your work! I'm a little confused by some of the changes to When initially reading about this upgrade to This would typically lead me to the discussions around the fact that This is where I presumed these changes came in attempting to unify the instance returned by However from my testing I'm struggling to understand how this works I have a simple unit test here that illustrates the problem: import type { NuxtApp } from '#app'
import type { AsyncDataRefreshCause } from '#app/composables/asyncData'
import { expect, it, vi } from 'vitest'
const handler = vi.fn(async () => Promise.resolve('hello'))
const getCachedData = vi.fn((key: string, nuxtApp: NuxtApp, ctx: { cause: AsyncDataRefreshCause }) => {
if (nuxtApp.isHydrating) {
return nuxtApp.payload.data[key]
}
const { data } = useNuxtData(key)
if (ctx.cause !== 'refresh:manual' && ctx.cause !== 'refresh:hook' && data.value) {
return data.value
}
})
// eslint-disable-next-line ts/promise-function-async
function testAsyncData() {
return useAsyncData('test-key', handler, {
getCachedData,
})
}
it('test duplicate calls are not made after first call has finished', async () => {
const { status, data } = await testAsyncData()
expect(status.value).toBe('success') // pass
expect(data.value).toBe('hello') // pass
expect(handler).toHaveBeenCalledTimes(1) // pass
const { status: status2, data: data2 } = testAsyncData()
expect.soft(handler).toHaveBeenCalledTimes(1) // fail - called twice
expect.soft(getCachedData).toHaveBeenCalledTimes(2)
expect.soft(data.value).toBe('hello') // pass
expect.soft(data2.value).toBe('hello') // pass
expect.soft(status.value).toBe('success') // fail - value is 'pending'
expect.soft(status2.value).toBe('success') // fail - value is 'pending'
})Effectively what I am trying to do here is to adjust the default However what actually happens is that whilst the nuxt/packages/nuxt/src/app/composables/asyncData.ts Lines 636 to 645 in ebff42f It is then used here but it only ever uses the At present it seems to be that we cannot stop the |
🔗 Linked issue
resolves #21532
resolves #24332 and therefore closes #25850
resolves #22348
resolves #27552
resolves #23522 and therefore closes #23993
resolves #27204
resolves #26733
partly implements #15438
📚 Description
This PR is a major reorganisation of the data fetching layer in Nuxt, providing performance/memory improvements + increased consistency.
1.
getCachedDatabehavior changeThe
getCachedDataoption now:watchorrefreshNuxtDatacauseproperty🐞 Bug Fixes
1. Shared refs for the same key
All calls to
useAsyncDataoruseFetchwith the same key will now share not just the underlying data but alsodata,error, andstatusrefs. This ensures consistency across components but may affect code that expected isolated instances.2. Warnings for inconsistent options
Multiple calls to
useAsyncDatawith the same key but different options will now trigger development warnings. The following options must be consistent across all calls with the same key:handlerfunctiondeepoptiontransformfunctionpickarraygetCachedDatafunctiondefaultvalueThe following options can differ without triggering warnings:
serverlazyimmediatededupewatch✨ New Features
1. Reactive keys
You can now use computed refs, plain refs or getter functions as keys, allowing for dynamic data fetching that automatically updates when dependencies change:
2. Deduped watch calls
Multiple components watching the same data source (like route changes) will now trigger only a single data refetch:
🔄 Migration Guide
For getCachedData users
If you were using
getCachedData, update your implementation to handle the new context parameter:Alternatively, for now, you can disable this behaviour with:
For duplicate key users
If you were intentionally using the same key in multiple places:
🧪 Testing Recommendations
When updating to this version:
🚧 TODO
getCachedData