Skip to content

Conversation

@edison1105
Copy link
Member

@edison1105 edison1105 commented Sep 26, 2025

fix #13732 (comment)
related PR #13560

Summary by CodeRabbit

  • Bug Fixes
    • Improved cleanup of Suspense fallbacks and placeholders during async rendering to reduce leftover DOM references and visual artifacts after transitions or unmounts.
    • More aggressive GC-friendly clearing of internal references when placeholders are removed or delayed enters complete, yielding more predictable mount/unmount behavior and less flicker. No public API changes.

@coderabbitai
Copy link

coderabbitai bot commented Sep 26, 2025

Walkthrough

Clear GC-sensitive vnode references in Suspense: remove fallback vnode el after transitions/unmounts when in fallback, clear vnode.placeholder before removing placeholder during async resolution, and expose isInFallback on the internal suspense boundary for these cleanups.

Changes

Cohort / File(s) Summary
Suspense state cleanup
packages/runtime-core/src/components/Suspense.ts
- Clear vnode.ssFallback.el after delayed-enter transition completes when still in fallback.
- Clear vnode.ssFallback.el after unmounting the active branch when no delayed enter and still in fallback.
- Set vnode.placeholder = null before removing the placeholder node when an async dep resolves.
- Expose isInFallback on the internal suspense boundary context for the above cleanups.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Async as Async dep / component
  participant Suspense as Suspense boundary
  participant Fallback as Fallback VNode (ssFallback)
  participant Placeholder as Placeholder VNode
  participant DOM as DOM

  Note over Suspense,Async: Async dependency resolves / transitions complete
  Async->>Suspense: resolve()
  Suspense->>Suspense: determine isInFallback
  alt delayed enter finished & in fallback
    Suspense->>Fallback: clear `el` reference (fallback.el = null)
    Suspense->>DOM: remove fallback/active nodes
  else unmount active branch & in fallback
    Suspense->>Fallback: clear `el` reference (fallback.el = null)
    Suspense->>DOM: remove active branch nodes
  end

  Note over Suspense,Placeholder: removing placeholder during resolve
  Suspense->>Placeholder: set `placeholder = null`
  Suspense->>DOM: remove placeholder node
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review focus:
    • packages/runtime-core/src/components/Suspense.ts — ensure clearing el/placeholder is safe across all suspend/resolve/unmount code paths.
    • Verify isInFallback exposure doesn't change external behavior or lifecycle assumptions.
    • Check transition/delayed-enter edge cases to avoid premature clears.

Possibly related PRs

Poem

I tidy hollows, nibble threads away,
Drop tiny links so memory can play.
Placeholder freed, fallback set loose,
Hoplight and nimble — a cleaner spruce. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR addresses the memory leak in #13732 by implementing cleanup logic for vnode references in Suspense, including resetting vnode.placeholder and clearing vnode.ssFallback.el, directly targeting the blockStack growth issue.
Out of Scope Changes check ✅ Passed All changes are focused on Suspense component cleanup and garbage collection, directly addressing the memory leak in #13732 without introducing unrelated modifications.
Title check ✅ Passed The title clearly and specifically describes the main changes: clearing placeholder and fallback element references after resolution to enable garbage collection, which directly matches the implemented cleanup logic.
✨ 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 edison/fix/asyncComponentPlaceholder

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

@github-actions
Copy link

github-actions bot commented Sep 26, 2025

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 103 kB (+120 B) 38.9 kB (+37 B) 35 kB (+53 B)
vue.global.prod.js 161 kB (+120 B) 58.8 kB (+33 B) 52.4 kB (+17 B)

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.9 kB 18.3 kB 16.8 kB
createApp 55 kB 21.4 kB 19.6 kB
createSSRApp 59.3 kB 23.1 kB 21.1 kB
defineCustomElement 60.6 kB 23.1 kB 21.1 kB
overall 69.3 kB (+120 B) 26.6 kB (+35 B) 24.3 kB (+16 B)

@pkg-pr-new
Copy link

pkg-pr-new bot commented Sep 26, 2025

Open in StackBlitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@13928

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@13928

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@13928

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@13928

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@13928

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@13928

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@13928

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@13928

@vue/shared

npm i https://pkg.pr.new/@vue/shared@13928

vue

npm i https://pkg.pr.new/vue@13928

@vue/compat

npm i https://pkg.pr.new/@vue/compat@13928

commit: cd706a4

@edison1105 edison1105 force-pushed the edison/fix/asyncComponentPlaceholder branch from bd5f7a5 to 588de48 Compare September 26, 2025 01:23
@edison1105 edison1105 added ready to merge The PR is ready to be merged. scope: suspense 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Sep 26, 2025
@edison1105
Copy link
Member Author

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented Nov 5, 2025

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools success success
pinia success success
primevue success success
nuxt success success
test-utils success success
quasar success success
radix-vue success success
vitepress success success
vant failure success
vite-plugin-vue success success
vue-macros success success
vuetify success success
router success success
vue-i18n failure failure
vueuse success success
vue-simple-compiler success success

@edison1105 edison1105 changed the title fix(Suspense): reset vnode.placeholder to null when removing placeholder fix(suspense): clear fallback and placeholder el refs to enable GC Nov 5, 2025
@edison1105 edison1105 changed the title fix(suspense): clear fallback and placeholder el refs to enable GC fix(suspense): clear placeholder and fallback el refs after resolved to enable GC Nov 5, 2025
@edison1105 edison1105 merged commit f411c66 into main Nov 5, 2025
17 of 18 checks passed
@edison1105 edison1105 deleted the edison/fix/asyncComponentPlaceholder branch November 5, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. ready to merge The PR is ready to be merged. scope: suspense

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Under special conditions, the blockStack will grow indefinitely, leading to a memory leak.

3 participants