feat: simplify useAsyncData() handler signature
#33629
Conversation
|
|
WalkthroughThe PR changes the AsyncDataHandler signature from two separate arguments to a single context object Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-09-10T14:42:56.647ZApplied to files:
📚 Learning: 2025-11-25T11:42:16.132ZApplied to files:
📚 Learning: 2024-12-12T12:36:34.871ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
OrbisK
force-pushed
the
feat/async-data-handler
branch
from
911b3cf to
2e570fc
Compare
CodSpeed Performance ReportMerging #33629 will not alter performanceComparing Summary
|
huang-julien
left a comment
huang-julien
left a comment
There was a problem hiding this comment.
Let's use a future flag to make the breaking change for v5
Can you help me with that? 😅 |
|
Another solution to reduce breaking change would be to count how many arguments are expected by the asyncData handler, if 2 expected, send the signal as 2nd argument, otherwise use the new signature. I am not sure that Nuxt users actually use the |
You mean like this? (handler.length) 
I think the second argument (the options ( |
|
@danielroe @atinux wdyt about abeaa68 
We can also do warnings in dev console or nuxt hints 🤔 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
800-802: Consider implementing handler.length detection for true backward compatibility.According to the PR discussion, @atinux suggested detecting the handler's parameter count to determine which signature to use. The current implementation passes both parameters but attempts to satisfy both signatures simultaneously, which leads to the mutation issue flagged above.
A cleaner approach would be:
🔎 Recommended implementation
-// keep old signature for backward compatibility, see https://github.com/nuxt/nuxt/pull/33629 -const ctx = Object.assign(nuxtApp, { signal: mergedSignal, nuxtApp }) -return Promise.resolve(handler(ctx, { signal: mergedSignal })).then(resolve, reject) +// Detect handler signature for backward compatibility +// Old: (nuxtApp, { signal }) => Promise<ResT> +// New: ({ signal, nuxtApp }) => Promise<ResT> +if (handler.length === 2) { + // Legacy signature + if (import.meta.dev) { + console.warn('[nuxt] [useAsyncData] The handler signature (nuxtApp, { signal }) is deprecated. Use ({ signal, nuxtApp }) instead.') + } + return Promise.resolve(handler(nuxtApp as any, { signal: mergedSignal })).then(resolve, reject) +} else { + // New signature + const ctx = { signal: mergedSignal, nuxtApp } + return Promise.resolve(handler(ctx as any, { signal: mergedSignal })).then(resolve, reject) +}Note: The commit message mentions "deprecation warnings for legacy usage", but no warnings are currently emitted. This approach adds them in development mode.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nuxt/src/app/composables/asyncData.ts(12 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/app/composables/asyncData.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nuxt/src/app/composables/asyncData.ts
🧠 Learnings (6)
📚 Learning: 2025-09-10T14:42:56.647Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichter
Repo: nuxt/nuxt PR: 31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,vue} : Follow standard TypeScript conventions and best practices
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
🧬 Code graph analysis (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
packages/nuxt/src/app/nuxt.ts (4)
NuxtApp(199-199)RuntimeNuxtHooks(36-58)NuxtSSRContext(60-85)NuxtPayload(87-98)
🔇 Additional comments (2)
packages/nuxt/src/app/composables/asyncData.ts (2)
22-94: Well-structured deprecation types.The deprecation interfaces are thoroughly documented with clear migration paths for each property. This approach provides excellent type-level guidance for developers migrating to the new signature.
732-741: LGTM!The prerender handler wrapper correctly implements the new signature, destructuring
nuxtAppfrom the context parameter and passing both arguments to the handler.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
800-802: Critical: Avoid mutating the shared nuxtApp instance.Line 801 uses
Object.assign(nuxtApp, ...)which directly mutates the sharednuxtAppinstance by addingsignalandnuxtAppproperties to it. This creates several problems:
- Side effects: Mutating the shared instance can cause unexpected behaviour if
nuxtAppis reused elsewhere- Circular reference:
nuxtApp.nuxtApp = nuxtAppcreates a circular reference- Race conditions: Concurrent handler invocations may interfere with each other
- Misleading comment: Line 800 states "keep old signature for backward compatibility", but this implements the new signature (context object as first parameter)
🔎 Proposed fix
Option 1: Shallow copy (simplest):
-// keep old signature for backward compatibility, see https://github.com/nuxt/nuxt/pull/33629 -const ctx = Object.assign(nuxtApp, { signal: mergedSignal, nuxtApp }) -return Promise.resolve(handler(ctx, { signal: mergedSignal })).then(resolve, reject) +// New context-based signature, see https://github.com/nuxt/nuxt/pull/33629 +const ctx = { ...nuxtApp, signal: mergedSignal, nuxtApp } +return Promise.resolve(handler(ctx, { signal: mergedSignal })).then(resolve, reject)Option 2: Preserve prototype chain:
-// keep old signature for backward compatibility, see https://github.com/nuxt/nuxt/pull/33629 -const ctx = Object.assign(nuxtApp, { signal: mergedSignal, nuxtApp }) -return Promise.resolve(handler(ctx, { signal: mergedSignal })).then(resolve, reject) +// New context-based signature, see https://github.com/nuxt/nuxt/pull/33629 +const ctx = Object.assign(Object.create(Object.getPrototypeOf(nuxtApp)), nuxtApp, { signal: mergedSignal, nuxtApp }) +return Promise.resolve(handler(ctx, { signal: mergedSignal })).then(resolve, reject)This issue was flagged in previous reviews and requires immediate attention before merging.
🧹 Nitpick comments (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
22-94: Deprecated types lack runtime detection and warnings.The deprecated interface
_DeprecatedAsyncDataHandlerNuxtAppand type_DeprecatedAsyncDataHandlerOptionsprovide helpful IDE hints, but the implementation doesn't include the runtime detection strategy discussed in the PR (usinghandler.lengthto distinguish old vs new signatures). Users will see deprecation warnings in their IDE, but there's no console warning or gradual migration path at runtime.Consider implementing the detection strategy mentioned in the PR discussion to reduce breaking changes and provide actionable warnings to developers.
Based on PR objectives discussion about backward compatibility strategy.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nuxt/src/app/composables/asyncData.ts(12 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/app/composables/asyncData.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nuxt/src/app/composables/asyncData.ts
🧠 Learnings (6)
📚 Learning: 2025-09-10T14:42:56.647Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichter
Repo: nuxt/nuxt PR: 31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,vue} : Follow standard TypeScript conventions and best practices
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
🧬 Code graph analysis (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
packages/nuxt/src/app/nuxt.ts (4)
NuxtApp(199-199)RuntimeNuxtHooks(36-58)NuxtSSRContext(60-85)NuxtPayload(87-98)
🔇 Additional comments (3)
packages/nuxt/src/app/composables/asyncData.ts (3)
2-2: LGTM: Import additions support new type definitions.The new imports for
App,RuntimeNuxtHooks,Hookable, andRuntimeConfigare necessary to properly type the deprecated handler context interface.Also applies to: 5-5, 15-16
96-96: LGTM: Type correctly defines new handler context.The
AsyncDataHandlerContexttype properly defines the new context shape with bothsignalandnuxtAppproperties. (Note: The typo flagged in previous reviews appears to have been resolved.)
732-741: LGTM: Prerender handler wrapper correctly adapted to new signature.The wrapper properly destructures
nuxtAppfrom the context parameter and forwards both the context and deprecated options to the handler whilst maintaining the shared prerender cache logic.
OrbisK
changed the title
useAsyncData() handler signatureuseAsyncData() handler signature
|
I like your solution in the commit. I will let @danielroe finally decide on it! |
|
|
||
| return Promise.resolve(handler(nuxtApp, { signal: mergedSignal })).then(resolve, reject) | ||
| // keep old signature for backward compatibility, see https://github.com/nuxt/nuxt/pull/33629 | ||
| const ctx = Object.assign(nuxtApp, { signal: mergedSignal, nuxtApp }) |
There was a problem hiding this comment.
this will mutate nuxtApp rather than creating a new object for the handler. how about let's try a proxy instead?
| // @ts-expect-error virtual file | ||
| import { asyncDataDefaults, granularCachedData, pendingWhenIdle, purgeCachedData } from '#build/nuxt.config.mjs' | ||
| import type { Hookable } from 'hookable' | ||
| import type { RuntimeConfig } from '../../../schema' |
There was a problem hiding this comment.
we should import from @nuxt/schema
| /** | ||
| * @deprecated Use `useAsyncData(key, ({nuxtApp})=>{...})` instead. | ||
| */ | ||
| interface _DeprecatedAsyncDataHandlerNuxtApp extends NuxtApp { |
There was a problem hiding this comment.
hm. I wonder if there is a simpler way to do this?
There was a problem hiding this comment.
I haven't found a way yet.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
96-99: Handler signature maintains backward compatibility at cost of complexity.The exported
AsyncDataHandlertype accepts both the new context object and deprecated options parameter, withsignalappearing in both places. This is intentional for the migration period but does add complexity rather than simplifying the signature.Consider documenting the migration path more explicitly, perhaps with a simplified type alias for new code:
// For new code (recommended) export type SimpleAsyncDataHandler<ResT> = (context: AsyncDataHandlerContext) => Promise<ResT>
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nuxt/src/app/composables/asyncData.ts(12 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/app/composables/asyncData.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nuxt/src/app/composables/asyncData.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
📚 Learning: 2025-09-10T14:42:56.647Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichter
Repo: nuxt/nuxt PR: 31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,vue} : Follow standard TypeScript conventions and best practices
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
🧬 Code graph analysis (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
packages/nuxt/src/app/nuxt.ts (4)
NuxtApp(199-199)RuntimeNuxtHooks(36-58)NuxtSSRContext(60-85)NuxtPayload(87-98)
🔇 Additional comments (2)
packages/nuxt/src/app/composables/asyncData.ts (2)
15-16: LGTM!The imports from
hookableand@nuxt/schemaare correctly added as type-only imports, addressing previous feedback to importRuntimeConfigfrom@nuxt/schema.
732-741: LGTM!The prerender handler wrapper correctly adapts to the new context-based signature, extracting
nuxtAppfrom the context for cache operations whilst forwarding the full context to the underlying handler.
| /** | ||
| * @deprecated Use `useAsyncData(key, ({nuxtApp})=>{...})` instead. | ||
| */ | ||
| interface _DeprecatedAsyncDataHandlerNuxtApp extends NuxtApp { | ||
| /** | ||
| * @deprecated use `useAsyncData(key, ({nuxtApp})=>{nuxtApp.vueApp})` instead. | ||
| */ | ||
| vueApp: App<Element> |
There was a problem hiding this comment.
Deprecation message syntax is misleading.
The deprecation messages use ({nuxtApp})=>{nuxtApp.vueApp} which looks like returning an object literal (invalid syntax). Consider using arrow function syntax without braces for clarity:
- * @deprecated use `useAsyncData(key, ({nuxtApp})=>{nuxtApp.vueApp})` instead.
+ * @deprecated use `useAsyncData(key, ({ nuxtApp }) => nuxtApp.vueApp)` instead.This pattern repeats throughout all the deprecation messages in lines 27, 31, 35, 39, 43, 47, 51, 56, 60, 64, 68, 72, 76, and 81.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @deprecated Use `useAsyncData(key, ({nuxtApp})=>{...})` instead. | |
| */ | |
| interface _DeprecatedAsyncDataHandlerNuxtApp extends NuxtApp { | |
| /** | |
| * @deprecated use `useAsyncData(key, ({nuxtApp})=>{nuxtApp.vueApp})` instead. | |
| */ | |
| vueApp: App<Element> | |
| /** | |
| * @deprecated Use `useAsyncData(key, ({nuxtApp})=>{...})` instead. | |
| */ | |
| interface _DeprecatedAsyncDataHandlerNuxtApp extends NuxtApp { | |
| /** | |
| * @deprecated use `useAsyncData(key, ({ nuxtApp }) => nuxtApp.vueApp)` instead. | |
| */ | |
| vueApp: App<Element> |
🤖 Prompt for AI Agents
In packages/nuxt/src/app/composables/asyncData.ts around lines 22 to 29, the
deprecation messages use misleading arrow-function-with-braces syntax like
`({nuxtApp})=>{nuxtApp.vueApp}` which appears to return an object literal;
replace these with correct concise arrow function syntax such as `({nuxtApp}) =>
nuxtApp.vueApp` (no braces, implicit return) and update the same pattern on the
other listed lines (27, 31, 35, 39, 43, 47, 51, 56, 60, 64, 68, 72, 76, 81) so
all deprecation messages show valid, clear example usage.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
22-84: Clarify deprecation message syntax.The deprecation messages use arrow functions with braces like
({nuxtApp})=>{nuxtApp.vueApp}, which could be misread as attempting to return an object literal. For clarity, consider using implicit return syntax without braces:({nuxtApp}) => nuxtApp.vueApp.This pattern appears throughout the deprecation messages (lines 27, 31, 35, 39, 43, 47, 51, 56, 60, 64, 68, 72, 76, 81).
🔎 Example fix for clearer syntax
/** - * @deprecated use `useAsyncData(key, ({nuxtApp})=>{nuxtApp.vueApp})` instead. + * @deprecated use `useAsyncData(key, ({nuxtApp}) => nuxtApp.vueApp)` instead. */ vueApp: App<Element>Apply the same pattern to all other deprecation messages in this interface.
Based on past review comment about misleading arrow function syntax.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nuxt/src/app/composables/asyncData.ts(12 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/app/composables/asyncData.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nuxt/src/app/composables/asyncData.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
📚 Learning: 2025-09-10T14:42:56.647Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Use clear, descriptive variable and function names
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,js,jsx,vue} : Use error handling patterns consistently
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.{ts,tsx,vue} : Follow standard TypeScript conventions and best practices
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichter
Repo: nuxt/nuxt PR: 31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Applied to files:
packages/nuxt/src/app/composables/asyncData.ts
🧬 Code graph analysis (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
packages/nuxt/src/app/nuxt.ts (4)
NuxtApp(199-199)RuntimeNuxtHooks(36-58)NuxtSSRContext(60-85)NuxtPayload(87-98)
🔇 Additional comments (5)
packages/nuxt/src/app/composables/asyncData.ts (5)
2-2: LGTM: Type imports are appropriate.The new type imports (
App,RuntimeNuxtHooks,Hookable,RuntimeConfig) are necessary to support the deprecated handler interface properties and are correctly imported as types.Also applies to: 5-5, 15-16
86-96: LGTM: Context types are well-defined.The
AsyncDataHandlerContexttype correctly captures the new simplified signature with bothsignalandnuxtAppin a single object, and the deprecated types appropriately document the legacy parameters.
232-232: LGTM: Overload signatures updated consistently.The trailing comma additions and references to the updated
AsyncDataHandlertype are appropriate and maintain consistency across all overloads.Also applies to: 242-242, 260-260, 271-271, 541-541, 551-551, 569-569, 580-580
732-741: LGTM: Prerender handler correctly uses new signature.The handler wrapper correctly destructures
nuxtAppfrom the context object and maintains backward compatibility by passing both the context and deprecated options parameters.
800-815: LGTM: Proxy implementation provides backward compatibility.The proxy correctly intercepts access to deprecated NuxtApp properties, logs warnings, and forwards them from the
nuxtAppobject. The implementation:
- Uses a
Setto log each deprecated property only once per handler invocation- Forwards deprecated properties like
vueApp,hooks,payload, etc., fromnuxtApp- Preserves direct access to the new
signalandnuxtApppropertiesThis approach successfully maintains backward compatibility whilst guiding users towards the new signature through deprecation warnings.
| export type AsyncDataHandlerContext = { signal: AbortSignal, nuxtApp: NuxtApp } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| export type AsyncDataHandler<ResT> = ((context: AsyncDataHandlerContext & _DeprecatedAsyncDataHandlerNuxtApp, options: _DeprecatedAsyncDataHandlerOptions) => Promise<ResT>) |
There was a problem hiding this comment.
Handler signature contradicts "simplify" objective.
The AsyncDataHandler type still requires two parameters (context and options), which contradicts the PR objective to simplify the handler signature. Whilst the runtime proxy implementation (lines 800-815) allows handlers to ignore the second parameter, the TypeScript signature forces all new handlers to accept both:
// Current (still complex):
(context: AsyncDataHandlerContext & _DeprecatedAsyncDataHandlerNuxtApp, options: _DeprecatedAsyncDataHandlerOptions) => Promise<ResT>
// Expected simplification:
(context: AsyncDataHandlerContext) => Promise<ResT>Consider making the second parameter optional in the type signature to reflect the actual simplified usage: (context: AsyncDataHandlerContext & _DeprecatedAsyncDataHandlerNuxtApp, options?: _DeprecatedAsyncDataHandlerOptions) => Promise<ResT>. This would allow new handlers to use just ({ signal, nuxtApp }) => ... whilst maintaining backward compatibility at the type level.
Based on PR objectives stating "simplify useAsyncData() handler signature" and past review comment about signature complexity.
🤖 Prompt for AI Agents
In packages/nuxt/src/app/composables/asyncData.ts around line 99, the
AsyncDataHandler type currently forces two parameters which contradicts the
simplification goal; change the signature to make the second parameter optional
so callers can use (context: AsyncDataHandlerContext &
_DeprecatedAsyncDataHandlerNuxtApp) => Promise<ResT> or include the legacy
options when needed, i.e. update the type to use options?:
_DeprecatedAsyncDataHandlerOptions while preserving existing intersection types
and exports; run a quick type-check to ensure no other type declarations need
adjustments.
…dler # Conflicts: # packages/nuxt/src/app/composables/asyncData.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/4.api/2.composables/use-async-data.md (1)
74-76: Update outdated terminology about handler arguments.Line 76 refers to the signal as being "provided in the second argument," but the new handler signature passes signal as part of a single context object. This text should be updated to match the new signature pattern shown in the code examples below it.
🔎 Proposed documentation update
- You can make your `handler` function abortable by using the `signal` provided in the second argument. This is useful for cancelling requests when they are no longer needed, such as when a user navigates away from a page. `$fetch` natively supports abort signals. + You can make your `handler` function abortable by using the `signal` provided in the context object passed to the handler. This is useful for cancelling requests when they are no longer needed, such as when a user navigates away from a page. `$fetch` natively supports abort signals.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/4.api/2.composables/use-async-data.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-10T14:42:56.647Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
Applied to files:
docs/4.api/2.composables/use-async-data.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: test-fixtures (windows-latest, built, webpack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: release-pkg-pr-new
- GitHub Check: test-size
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: test-benchmark
- GitHub Check: code
🔇 Additional comments (4)
docs/4.api/2.composables/use-async-data.md (4)
19-26: Handler signature correctly updated to single-context pattern.The basic usage example accurately reflects the new handler signature with signal destructuring from the context parameter.
40-55: Watch Params example correctly uses new handler signature.The example demonstrates the new context destructuring pattern in conjunction with the watch option, maintaining clarity and consistency.
176-203: Shared State examples correctly reflect new handler signature.The consistency rule examples accurately use the updated handler signature pattern with signal destructuring. The pedagogy remains clear in demonstrating which options must align across keyed calls.
230-281: Type signature accurately documents the new handler context shape.The type definition correctly consolidates handler parameters into a single context object containing both
signalandnuxtApp. The overload signatures and related types are properly aligned with this change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/4.api/2.composables/use-async-data.md (1)
197-202: Add example demonstratingnuxtAppusage from context.The type signature (line 233) shows
nuxtAppis available in the context object alongsidesignal, but none of the code examples demonstrate how to access it. Consider adding a brief example showing how to destructure and usenuxtAppif developers need it.Example snippet:
const { data } = await useAsyncData( 'example', ({ signal, nuxtApp }) => { // Both signal and nuxtApp are available in the context return $fetch('/api/data', { signal }) } )
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/4.api/2.composables/use-async-data.md
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
📚 Learning: 2025-09-10T14:42:56.647Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
Applied to files:
docs/4.api/2.composables/use-async-data.md
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/4.api/2.composables/use-request-fetch.md (1)
32-36: Handler signatures are consistent and correct, but lack migration context.Both examples correctly demonstrate handler usage: line 32 omits the handler context parameter (appropriate for requestFetch), and line 36 properly destructures and forwards the signal. However, readers upgrading from Nuxt 4 will find no explanation that this signature changed. A brief note in the tip or note block would help.
Consider adding a note explaining that the handler now receives a single context object containing signal and nuxtApp, especially since this is a user-facing breaking change.
docs/1.getting-started/10.data-fetching.md (1)
197-207: All examples are technically correct, but critical breaking change context is missing.The handler examples consistently show the new single-argument context signature with proper signal destructuring and forwarding. However, this is the primary getting-started guide for data fetching — an ideal place to highlight the breaking change from the previous two-argument handler signature.
Add a prominent breaking change notice or callout near the beginning of the "useAsyncData" section (around line 153) that explains the handler signature change. Users upgrading from earlier Nuxt 4.x versions need to understand this breaking change upfront. Consider phrasing like:
"In Nuxt 4.x and later, the handler signature changed from
(nuxtApp, { signal })to({ signal, nuxtApp }). All signal references must be destructured from the single context parameter."or reference a migration section if available elsewhere.
Also applies to: 375-398, 807-812
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/1.getting-started/10.data-fetching.mddocs/4.api/2.composables/use-nuxt-app.mddocs/4.api/2.composables/use-nuxt-data.mddocs/4.api/2.composables/use-request-fetch.mddocs/4.api/3.utils/$fetch.mddocs/7.migration/7.component-options.md
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-10T14:42:56.647Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 33192
File: test/nuxt/use-async-data.test.ts:366-373
Timestamp: 2025-09-10T14:42:56.647Z
Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
Applied to files:
docs/7.migration/7.component-options.mddocs/4.api/2.composables/use-request-fetch.mddocs/4.api/3.utils/$fetch.mddocs/4.api/2.composables/use-nuxt-app.mddocs/4.api/2.composables/use-nuxt-data.mddocs/1.getting-started/10.data-fetching.md
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Applied to files:
docs/4.api/3.utils/$fetch.mddocs/4.api/2.composables/use-nuxt-app.mddocs/4.api/2.composables/use-nuxt-data.md
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
docs/4.api/2.composables/use-nuxt-data.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: codeql (javascript-typescript)
- GitHub Check: build
- GitHub Check: codeql (actions)
- GitHub Check: code
🔇 Additional comments (4)
docs/4.api/3.utils/$fetch.md (1)
31-31: All handler examples correctly demonstrate the new signature.The examples on lines 31, 78, and 807 consistently and correctly show handlers receiving a single context object with destructured signal, which is properly forwarded to $fetch. This is pedagogically clear and accurate.
Also applies to: 78-78, 807-807
docs/4.api/2.composables/use-nuxt-app.md (1)
111-111: Example correctly demonstrates the updated handler signature.The example properly shows the useAsyncData handler with destructured signal and its forwarding to $fetch. This aligns with all other documentation updates in the PR.
docs/4.api/2.composables/use-nuxt-data.md (1)
70-70: Fix the useAsyncData handler signature in the example.The example shows an incomplete handler signature. The handler should receive two parameters: the NuxtApp instance and an options object containing the signal. The current example
({ signal }) => ...omits the first parameter.Correct to:
(_nuxtApp, { signal }) => $fetch('/api/todos', { signal })⛔ Skipped due to learnings
Learnt from: Tofandel Repo: nuxt/nuxt PR: 33192 File: test/nuxt/use-async-data.test.ts:366-373 Timestamp: 2025-09-10T14:42:56.647Z Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.docs/7.migration/7.component-options.md (1)
51-51: Handler signature in the example is incomplete.The example shows
({ signal }) => $fetch(...)but is missing the first handler parameter. Either use the simple signature() => $fetch(...)or the full signature(_nuxtApp, { signal }) => $fetch(..., { signal })where signal is needed for request cancellation.⛔ Skipped due to learnings
Learnt from: Tofandel Repo: nuxt/nuxt PR: 33192 File: test/nuxt/use-async-data.test.ts:366-373 Timestamp: 2025-09-10T14:42:56.647Z Learning: In the Nuxt useAsyncData test "should watch params deeply in a non synchronous way", the foo watcher intentionally updates both params.foo and params.locale using locale.value, simulating a scenario where one watcher consolidates multiple reactive values into a shared params object for testing debounced/non-synchronous behavior.
4 participants
🔗 Linked issue
fixes #33623
📚 Description
This simplifies the usage for
useAsyncDatahandlers.