Skip to content

Conversation

edison1105
Copy link
Member

@edison1105 edison1105 commented Oct 4, 2025

close #13955

Summary by CodeRabbit

  • Bug Fixes

    • Improved slot aggregation for custom elements without shadow DOM: slots are now collected across all root contexts (including teleport targets) and deduplicated, preventing duplicate slot entries and ensuring more consistent slot content rendering in complex layouts.
  • Tests

    • Added a test validating teleport targets that are ancestors of a custom element host, confirming correct rendering boundaries and cleanup on unmount.

Copy link

coderabbitai bot commented Oct 4, 2025

Walkthrough

Updates VueElement._getSlots to iterate all root nodes (host plus Teleport targets), collect slot elements into a Set to deduplicate across roots, and return the aggregated array, preventing duplicate slot handling when Teleport targets overlap with the custom element host.

Changes

Cohort / File(s) Summary
Custom element slot collection
packages/runtime-dom/src/apiCustomElement.ts
Rewrote _getSlots to iterate over all root nodes (host + teleport targets), query slot elements per root, insert them into a Set to deduplicate, and return Array.from(set) instead of concatenating per-root arrays.
Tests: Teleport + custom element
packages/runtime-dom/__tests__/customElement.spec.ts
Added test "teleport target is ancestor of custom element host" to verify Teleport behavior when the Teleport target is an ancestor of the custom element host, asserting expected DOM and successful unmount without slot render errors.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Browser
  participant CE as VueCustomElement
  participant Roots as Root Nodes (host + Teleport targets)
  participant Set as Slot Set

  Browser->>CE: mount custom element (shadowRoot: false)
  CE->>Roots: discover root nodes (host + teleport targets)
  loop for each root
    CE->>Roots: query slot elements
    Roots-->>Set: add slot elements (deduplicated)
  end
  CE->>CE: Array.from(Set) -> slots array
  CE->>Browser: render slots (deduplicated)
  note right of CE: prevents null insertBefore when teleport target is ancestor
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I hopped through roots with twitching whiskers bright,
Gathered stray slots in a Set by moonlight.
No doubles now tumble, no insertBefore fright —
A tidy render, swift and light. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title concisely and clearly summarizes the main change by indicating that slot retrieval in custom elements is optimized to avoid duplicates. It directly reflects the code modification, which switches to a Set-based approach to dedupe slot elements. The title follows conventional commit style and provides a clear, specific description of the update.
Linked Issues Check ✅ Passed The pull request addresses the linked issue by updating the slot retrieval logic to handle multiple roots including teleport targets and prevent null-node errors when the teleport target is an ancestor of the host, and it adds a new test validating this scenario. This change directly prevents the crash in _renderSlots described in issue #13955 and ensures slotted content is reliably retrieved without duplicates or null insertBefore operations. The test confirms the expected behavior when the teleport target is an ancestor, satisfying the issue requirements.
Out of Scope Changes Check ✅ Passed All code modifications are restricted to the custom element slot retrieval logic and its corresponding test suite, directly targeting the behavior described in the linked issue. There are no unrelated or extraneous changes in other modules or public API declarations. The pull request remains focused on fixing the duplicate slot retrieval and crash scenario without introducing out-of-scope updates.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ 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/13955

📜 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 b661cd3 and 72b5019.

📒 Files selected for processing (1)
  • packages/runtime-dom/src/apiCustomElement.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/runtime-dom/src/apiCustomElement.ts

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

Copy link

github-actions bot commented Oct 4, 2025

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 102 kB (+42 B) 38.6 kB (+5 B) 34.7 kB (-16 B)
vue.global.prod.js 160 kB (+42 B) 58.7 kB (+7 B) 52.2 kB (-15 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 (+42 B) 23 kB (+16 B) 21 kB (+4 B)
overall 68.8 kB 26.5 kB 24.2 kB

Copy link

pkg-pr-new bot commented Oct 4, 2025

Open in StackBlitz

@vue/compiler-core

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

@vue/compiler-dom

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

@vue/compiler-sfc

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

@vue/compiler-ssr

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

@vue/reactivity

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

@vue/runtime-core

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

@vue/runtime-dom

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

@vue/server-renderer

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

@vue/shared

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

vue

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

@vue/compat

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

commit: 72b5019

@edison1105 edison1105 added 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. scope: custom elements labels Oct 4, 2025
@edison1105 edison1105 added the ready to merge The PR is ready to be merged. label Oct 4, 2025
@edison1105 edison1105 removed this from Next Minor Oct 4, 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: custom elements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[custom-element] Crash in _renderSlots when Teleport target is ancestor of custom element host (shadowRoot: false)
1 participant