Skip to content

fix(nuxt): remove abort signal event listeners after render #33665

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.

Sign up for GitHub

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

Jump to bottom
Merged
danielroe merged 1 commit into from
Nov 6, 2025
Merged

fix(nuxt): remove abort signal event listeners after render #33665

danielroe merged 1 commit into from
Nov 6, 2025

Conversation

@danielroe
Copy link
Member

@danielroe danielroe commented Nov 6, 2025 •
edited
Loading

🔗 Linked issue

resolves #33644
resolves #33080

📚 Description

we were registering some event listeners in #32918.

this cleans them up in either app:error or app:rendered

@bolt-new-by-stackblitz
Copy link

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

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 6, 2025

Open in StackBlitz

@nuxt/kit

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

nuxt

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

@nuxt/rspack-builder

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

@nuxt/schema

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

@nuxt/vite-builder

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

@nuxt/webpack-builder

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

commit: aec207d

@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025 •
edited
Loading

Walkthrough

The mergeAbortSignals function in the asyncData composable has been refactored to accept a NuxtApp parameter as its first argument. All existing call sites have been updated to pass this parameter. A new helper function, removeListenerAfterRender, has been introduced to manage abort listener cleanup on the server side via hooks for app rendering completion or error states. The abort handling logic now uses a dedicated callback mechanism and ensures proper cleanup of abort listeners post-render.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10–15 minutes

  • Verify that all call sites to mergeAbortSignals throughout the file have been consistently updated with the nuxtApp parameter
  • Confirm the removeListenerAfterRender helper correctly registers and fires cleanup on both 'app:rendered' and 'app:error' hooks
  • Check for potential race conditions between listener registration and removal during server rendering
  • Ensure the abort callback wiring integrates correctly with the existing signal merging logic

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 (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: removing abort signal event listeners after render, which directly addresses the memory leak issue.
Linked Issues check ✅ Passed The PR directly addresses the memory leak [#33644] by implementing cleanup of abort signal event listeners after render, which aligns with the identified root cause.
Out of Scope Changes check ✅ Passed All changes are scoped to abort signal listener management in asyncData.ts and directly target the memory leak issue without introducing unrelated modifications.
Description check ✅ Passed The pull request description clearly relates to the changeset by explaining that it cleans up event listeners registered in a previous PR, addressing the memory leak issue.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/remove-listeners

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.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 6, 2025

CodSpeed Performance Report

Merging #33665 will not alter performance

Comparing fix/remove-listeners (aec207d) with main (0014826)1

Summary

✅ 10 untouched

Footnotes

  1. No successful run was found on main (dbea282) during the generation of this report, so 0014826 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@danielroe danielroe merged commit 3e1eb66 into main Nov 6, 2025
58 checks passed
@danielroe danielroe deleted the fix/remove-listeners branch November 6, 2025 10:33
This was referenced Nov 6, 2025
Open
Merged
@OrbisK OrbisK mentioned this pull request Nov 6, 2025
Merged
danielroe added a commit that referenced this pull request Nov 6, 2025
@danielroe
fix(nuxt): remove abort signal event listeners after render (#33665)
286a18a
@github-actions github-actions bot mentioned this pull request Nov 6, 2025
Merged
@coderabbitai coderabbitai bot mentioned this pull request Jan 13, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

4.x bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Memory leak in Nuxt 4.2.0 EffectScope memory leak in Nuxt 4, when Vue setup blocks have await calls in them

2 participants

@danielroe