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 placeholders so they’re cleared before removal, reducing rare leftover nodes and visual artifacts during async rendering. Results in more predictable mount/unmount behavior and less flicker with nested or rapidly changing async content. No public API changes.

Copy link

coderabbitai bot commented Sep 26, 2025

Walkthrough

The Suspense component’s registerDep logic was adjusted to nullify vnode.placeholder before removing the corresponding DOM node. No public APIs or signatures were changed.

Changes

Cohort / File(s) Summary
Suspense placeholder state update
packages/runtime-core/src/components/Suspense.ts
In registerDep, sets vnode.placeholder = null prior to removing the placeholder node, clearing the placeholder reference before DOM removal.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Component (async dep)
  participant S as Suspense.registerDep
  participant V as VNode
  participant D as DOM

  Note over S,V: Handling completion of async dependency
  C->>S: resolve dependency
  S->>V: check V.placeholder
  alt Placeholder exists
    S->>V: set placeholder = null
    S->>D: remove placeholder node
  else No placeholder
    S->>S: continue
  end
  S->>C: proceed with resolution
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

scope: suspense, :hammer: p3-minor-bug

Poem

I nudge a node, then set it free,
A placeholder’s gone—no ghost for me.
With tidy paws I clear the trail,
One hop ahead, I will not fail.
Suspense exhaled, the scene made right. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The changes focus on resetting the Suspense vnode.placeholder reference but fail to address the blockStack growth or reset required by issue #13732, so the primary objective of clearing blockStack to prevent the memory leak remains unfulfilled. In addition to resetting vnode.placeholder, the implementation should include logic to clear or reset the runtime-core blockStack array after the relevant operations so that its length returns to zero as required by issue #13732.
Out of Scope Changes Check ⚠️ Warning The PR’s modification of Suspense.ts to clear vnode.placeholder is unrelated to the blockStack memory leak fix outlined in the linked issue, indicating that the change falls outside the scope of the objectives for preventing blockStack growth. This PR should be refocused to implement the blockStack reset logic required by issue #13732 or the Suspense placeholder change should be moved to a separate PR to keep the scope aligned with the memory leak fix.
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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the specific change made in the Suspense component by resetting vnode.placeholder to null when removing the placeholder, matching the code modifications and clearly indicating the main fix.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch edison/fix/asyncComponentPlaceholder

📜 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 bd5f7a5 and 588de48.

📒 Files selected for processing (1)
  • packages/runtime-core/src/components/Suspense.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/runtime-core/src/components/Suspense.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). (3)
  • GitHub Check: Redirect rules
  • GitHub Check: Header rules
  • GitHub Check: Pages changed

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

Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 102 kB (+21 B) 38.6 kB (+4 B) 34.8 kB (-7 B)
vue.global.prod.js 160 kB (+21 B) 58.7 kB (+6 B) 52.3 kB (+51 B)

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.7 kB 18.3 kB 16.7 kB
createApp 54.7 kB 21.3 kB 19.5 kB
createSSRApp 58.9 kB 23 kB 21 kB
defineCustomElement 60 kB 23 kB 21 kB
overall 68.8 kB (+21 B) 26.5 kB (+5 B) 24.1 kB (-52 B)

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: 588de48

@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
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.
1 participant