fix(suspense): defer clearing fallback vnode el in case it has dirs #14080

linzhe141 · 2025-11-12T07:21:53Z

close #14078

issue playground

playground with this pr

Summary by CodeRabbit

Bug Fixes
- Suspense fallback cleanup now occurs after the next render frame instead of immediately, improving stability and consistency during fallback-to-resolved transitions and unmounting.
Tests
- Added a test ensuring a directive on a fallback is invoked exactly once when the fallback is replaced by a resolved async component.

…unt active branch first

coderabbitai · 2025-11-12T07:22:09Z

Walkthrough

Defers clearing of vnode.ssFallback.el by scheduling the clear via queuePostRenderEffect when unmounting the active branch; adds a test ensuring an unmounted directive is invoked exactly once when a fallback is replaced by a resolved async component.

Changes

Cohort / File(s)	Summary
Suspense cleanup change `packages/runtime-core/src/components/Suspense.ts`	Import `queuePostRenderEffect` and replace synchronous clearing of `vnode.ssFallback.el` with a scheduled post-render clear in unmount/cleanup paths.
Test: directive/unmount behavior `packages/runtime-core/__tests__/components/Suspense.spec.ts`	Add a test asserting an unmounted directive is invoked exactly once when a fallback is replaced by a resolved async component; update imports to include `withDirectives`.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as Application
  participant Suspense as Suspense
  participant Renderer as Renderer
  participant PostRender as Post-Render Queue

  rect rgb(248,249,255)
  note over Suspense,Renderer: Before (synchronous)
  App->>Suspense: resolve async branch / unmount active branch
  Suspense->>Renderer: clear vnode.ssFallback.el (immediate)
  Renderer->>Renderer: continue render
  end

  rect rgb(240,255,240)
  note over Suspense,PostRender: After (deferred)
  App->>Suspense: resolve async branch / unmount active branch
  Suspense->>PostRender: queuePostRenderEffect(clear vnode.ssFallback.el, suspense)
  Suspense->>Renderer: continue render
  PostRender->>Suspense: run queued effect after render -> clear vnode.ssFallback.el
  end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Review packages/runtime-core/src/components/Suspense.ts for all unmount/cleanup branches to ensure consistent deferred clearing.
Verify queuePostRenderEffect usage and the passed suspense context.
Confirm the new test covers the edge case and imports are correct.

Possibly related PRs

fix(suspense): clear placeholder and fallback el refs after resolved to enable GC #13928 — Changes to Suspense cleanup timing for clearing fallback vnode element references (strong overlap).
fix(runtime-core): ensure correct anchor el for unresolved async components #13560 — Related adjustments to Suspense placeholder/anchor handling and unresolved async component behavior.

Suggested labels

ready to merge, scope: suspense, :hammer: p3-minor-bug

Poem

🐰 I hopped through code at morning light,

Deferred a clear to keep things right,
Queued my step till render's done,
One unmount call — not two, but one,
A gentle hop, the tests unite.

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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR addresses the linked issue #14078 by fixing the null reference error through deferred clearing of fallback vnode references and includes a test case verifying unmount behavior with directives.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the Suspense unmount timing issue: modifications to Suspense.ts defer the vnode reference clearing, and the test addition verifies the fix works correctly with directives.
Title check	✅ Passed	The title accurately describes the main change: deferring the clearing of the fallback vnode element reference to ensure proper cleanup when directives are present.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

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

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

github-actions · 2025-11-12T07:23:06Z

Size Report

Bundles

File	Size	Gzip	Brotli
runtime-dom.global.prod.js	103 kB (+8 B)	38.9 kB (+26 B)	35 kB (-39 B)
vue.global.prod.js	161 kB (+8 B)	58.8 kB (+22 B)	52.4 kB (+46 B)

Usages

Name	Size	Gzip	Brotli
createApp (CAPI only)	46.9 kB (+4 B)	18.3 kB (-2 B)	16.8 kB (+3 B)
createApp	55 kB	21.4 kB (-7 B)	19.6 kB (-10 B)
createSSRApp	59.3 kB	23.1 kB (-9 B)	21.1 kB (-9 B)
defineCustomElement	60.6 kB	23.1 kB (-5 B)	21.1 kB (+32 B)
overall	69.3 kB (+8 B)	26.6 kB (-9 B)	24.3 kB (-8 B)

pkg-pr-new · 2025-11-12T07:23:17Z

Open in StackBlitz

@vue/compiler-core

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

@vue/compiler-dom

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

@vue/compiler-sfc

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

@vue/compiler-ssr

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

@vue/reactivity

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

@vue/runtime-core

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

@vue/runtime-dom

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

@vue/server-renderer

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

@vue/shared

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

vue

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

@vue/compat

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

commit: 0a9d60a

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/runtime-core/src/components/Suspense.ts (1)
583-587: LGTM: Cleanup correctly deferred for non-transition case.

This appropriately defers the fallback vnode el cleanup for the case when there's no delayed transition (!delayEnter). The conditions are mutually exclusive with the first clear (lines 557-561), ensuring the cleanup happens exactly once via the appropriate code path.

Minor observation: The comment on line 582 mentions "only clear immediately" but the implementation now correctly defers to post-render. The comment could be updated to reflect this, though the code itself is correct.

Consider updating the comment to reflect the deferred execution:
-          // clear el reference from fallback vnode to allow GC
-          // only clear immediately if there's no delayed transition
+          // clear el reference from fallback vnode to allow GC after render
+          // only clear if there's no delayed transition (handled in afterLeave callback otherwise)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9c676f and 5e4df55.

📒 Files selected for processing (1)

packages/runtime-core/src/components/Suspense.ts (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

packages/runtime-core/src/components/Suspense.ts (1)

packages/runtime-core/src/renderer.ts (1)

queuePostRenderEffect (287-296)

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

GitHub Check: test / e2e-test

🔇 Additional comments (2)

packages/runtime-core/src/components/Suspense.ts (2)

23-23: LGTM: Import addition is correct.

The queuePostRenderEffect import is necessary for deferring the fallback vnode cleanup to the post-render phase, which addresses the null reference timing issue.

557-561: LGTM: Cleanup correctly deferred for transition case.

This properly defers clearing the fallback vnode's el reference until after the render phase completes. By scheduling the cleanup in queuePostRenderEffect within the transition's afterLeave callback, the element reference remains available throughout the transition and render operations, preventing the null reference error reported in issue #14078.

edison1105 · 2025-11-12T12:31:38Z

/ecosystem-ci run

vue-bot · 2025-11-12T12:37:38Z

📝 Ran ecosystem CI: Open

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

fix(suspense): defer clearing fallback vnode reference to should unmo…

5e4df55

…unt active branch first

coderabbitai bot reviewed Nov 12, 2025

View reviewed changes

linzhe141 and others added 2 commits November 12, 2025 17:40

chore: add unit test

f6ec775

chore: tweaks

0a9d60a

edison1105 added scope: suspense 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Nov 12, 2025

edison1105 changed the title ~~fix(suspense): defer clearing fallback vnode reference to should unmount active branch first~~ fix(suspense): defer clearing fallback vnode el in case it has dirs Nov 12, 2025

edison1105 added the ready to merge The PR is ready to be merged. label Nov 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(suspense): defer clearing fallback vnode el in case it has dirs #14080

fix(suspense): defer clearing fallback vnode el in case it has dirs #14080

linzhe141 commented Nov 12, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 12, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 12, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Nov 12, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

edison1105 commented Nov 12, 2025

Uh oh!

vue-bot commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

fix(suspense): defer clearing fallback vnode el in case it has dirs #14080

Are you sure you want to change the base?

fix(suspense): defer clearing fallback vnode el in case it has dirs #14080

Conversation

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

Uh oh!

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Pre-merge checks and finishing touches

Uh oh!

github-actions bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report

Bundles

Usages

Uh oh!

pkg-pr-new bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

edison1105 commented Nov 12, 2025

Uh oh!

vue-bot commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

linzhe141 commented Nov 12, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 12, 2025 •

edited

Loading

github-actions bot commented Nov 12, 2025 •

edited

Loading

pkg-pr-new bot commented Nov 12, 2025 •

edited

Loading