fix(nuxt): cleanup event listener with cleanup signal #33667

OrbisK · 2025-11-06T11:57:13Z

🔗 Linked issue

followup on #33665

📚 Description

It reworks the cleanup to use a signal to cleanup the eventlisteners.

bolt-new-by-stackblitz · 2025-11-06T11:57:17Z

Run & review this pull request in StackBlitz Codeflow.

packages/nuxt/src/app/composables/asyncData.ts

coderabbitai · 2025-11-06T12:00:03Z

Walkthrough

The changes refactor abort-signal management in the createAsyncData composable by introducing a dedicated cleanupController (AbortController) to centralise cleanup. mergeAbortSignals is reworked to accept a cleanupSignal parameter and no longer depends on nuxtApp. A single abort listener is registered on the merged signal and scoped to cleanupController.signal; the previous onAbort/removeListenerAfterRender pattern is removed. cleanupController.abort() is invoked in the finally path to trigger cleanup and ensure listeners are removed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Review mergeAbortSignals signature change and all updated call sites to ensure parameters are passed in the correct order and semantics.
Verify merged signal propagation and that the single abort listener behaves identically to the previous multi-listener approach.
Confirm cleanupController.abort() is invoked on all finalisation paths and does not interfere with normal handler logic.
Check for any remaining references to the removed removeListenerAfterRender or old onAbort pattern.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: implementing cleanup of event listeners using a cleanup signal, which directly aligns with the PR's core objective of refactoring the cleanup mechanism.
Description check	✅ Passed	The description is related to the changeset, referencing the follow-up to issue #33665 and explaining the rework to use a signal for cleaning up event listeners, which matches the implemented changes.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b76d45 and c08b3d2.

📒 Files selected for processing (1)

packages/nuxt/src/app/composables/asyncData.ts (4 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Follow standard TypeScript conventions and best practices

Files:

packages/nuxt/src/app/composables/asyncData.ts

🧠 Learnings (1)

📚 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

⏰ 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). (19)

GitHub Check: test-fixtures (windows-latest, built, webpack, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (windows-latest, built, vite-env-api, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, rspack, default, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, webpack, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, vite-env-api, default, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, vite-env-api, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite-env-api, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, lts/-1)
GitHub Check: release-pkg-pr-new
GitHub Check: test-benchmark
GitHub Check: typecheck (windows-latest, bundler)
GitHub Check: typecheck (ubuntu-latest, bundler)
GitHub Check: code

🔇 Additional comments (6)

packages/nuxt/src/app/composables/asyncData.ts (6)

706-706: LGTM! Clean separation of concerns.

The dedicated cleanup controller is well-scoped and provides a clear mechanism for managing event listener lifecycle independently of request abortion.

711-711: Excellent! Addresses previous review feedback.

The updated call site now passes cleanupController.signal instead of relying on nuxtApp, which aligns with danielroe's previous review comment. Based on past review comments.

717-720: LGTM! Modern and idiomatic cleanup pattern.

The use of { once: true, signal: cleanupController.signal } is the recommended approach for automatic listener cleanup. This ensures the listener is removed when cleanupController.abort() is invoked, preventing memory leaks.

774-774: LGTM! Proper cleanup in finally block.

Invoking cleanupController.abort() in the finally block ensures that event listeners are cleaned up regardless of whether the promise succeeds, fails, or is cancelled.

826-826: Excellent refactoring! Cleaner function signature.

Removing the nuxtApp dependency and accepting cleanupSignal directly makes the function more focused, testable, and addresses danielroe's previous review comment. Based on past review comments.

864-864: LGTM! Consistent cleanup pattern in polyfill.

The polyfill implementation correctly uses the cleanupSignal to scope event listeners, maintaining consistency with the approach used elsewhere. The optional chaining provides good defensive programming.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

pkg-pr-new · 2025-11-06T12:03:56Z

Open in StackBlitz

@nuxt/kit

npm i https://pkg.pr.new/@nuxt/kit@33667

nuxt

npm i https://pkg.pr.new/nuxt@33667

@nuxt/rspack-builder

npm i https://pkg.pr.new/@nuxt/rspack-builder@33667

@nuxt/schema

npm i https://pkg.pr.new/@nuxt/schema@33667

@nuxt/vite-builder

npm i https://pkg.pr.new/@nuxt/vite-builder@33667

@nuxt/webpack-builder

npm i https://pkg.pr.new/@nuxt/webpack-builder@33667

commit: c08b3d2

codspeed-hq · 2025-11-06T12:11:57Z

CodSpeed Performance Report

Merging #33667 will improve performances by 14.25%

_{Comparing OrbisK:fix/add-eventlistener-signal (c08b3d2) with main (3e1eb66)}

Summary

⚡ 1 improvement
✅ 9 untouched

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`writeTypes in the basic-types fixture`	91.4 ms	80 ms	+14.25%

fix(nuxt): cleanup event listener with cleanup signal

0b76d45

OrbisK requested a review from danielroe as a code owner November 6, 2025 11:57

github-actions bot added 4.x bug labels Nov 6, 2025

danielroe reviewed Nov 6, 2025

View reviewed changes

packages/nuxt/src/app/composables/asyncData.ts Outdated Show resolved Hide resolved

chore: remove nuxtApp parameter from mergeAbortSignals function

c08b3d2

OrbisK force-pushed the fix/add-eventlistener-signal branch from cdea624 to c08b3d2 Compare November 6, 2025 12:00

OrbisK mentioned this pull request Nov 6, 2025

Memory leak in Nuxt 4.2.0 #33644

Closed

danielroe approved these changes Nov 6, 2025

View reviewed changes

danielroe merged commit a1b6753 into nuxt:main Nov 6, 2025
54 of 55 checks passed

github-actions bot mentioned this pull request Nov 6, 2025

v4.2.1 #33565

Merged

danielroe pushed a commit that referenced this pull request Nov 6, 2025

fix(nuxt): cleanup event listener with cleanup signal (#33667)

8f718bc

github-actions bot mentioned this pull request Nov 6, 2025

v3.20.1 #33649

Merged

coderabbitai bot mentioned this pull request Dec 20, 2025

feat: simplify useAsyncData() handler signature #33629

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(nuxt): cleanup event listener with cleanup signal #33667

fix(nuxt): cleanup event listener with cleanup signal #33667

Uh oh!

OrbisK commented Nov 6, 2025

Uh oh!

bolt-new-by-stackblitz bot commented Nov 6, 2025

Uh oh!

Uh oh!

coderabbitai bot commented Nov 6, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Nov 6, 2025

Uh oh!

codspeed-hq bot commented Nov 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

fix(nuxt): cleanup event listener with cleanup signal #33667

fix(nuxt): cleanup event listener with cleanup signal #33667

Uh oh!

Conversation

OrbisK commented Nov 6, 2025

🔗 Linked issue

📚 Description

Uh oh!

bolt-new-by-stackblitz bot commented Nov 6, 2025

Uh oh!

Uh oh!

coderabbitai bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

pkg-pr-new bot commented Nov 6, 2025

Uh oh!

codspeed-hq bot commented Nov 6, 2025

CodSpeed Performance Report

Merging #33667 will improve performances by 14.25%

Summary

Benchmarks breakdown

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Nov 6, 2025 •

edited

Loading