Skip to content

fix(ListboxVirtualizer): ignore non-element VNodes in slot children#2578

Merged
zernonia merged 1 commit into
unovue:v2from
zernonia:fix/listbox-virtualizer-comment-nodes
Apr 6, 2026
Merged

fix(ListboxVirtualizer): ignore non-element VNodes in slot children#2578
zernonia merged 1 commit into
unovue:v2from
zernonia:fix/listbox-virtualizer-comment-nodes

Conversation

@zernonia

@zernonia zernonia commented Apr 6, 2026

Copy link
Copy Markdown
Member

Summary

  • Fixes ListboxVirtualizer slot rendering when comment or text nodes are present (e.g. <!-- @vue-expect-error -->)
  • Replaces children[0] with a .find() that skips non-element VNodes (Comment, Text, Static, Fragment) by checking typeof type !== 'symbol'

Closes #2563

Test plan

  • Verify ComboboxVirtualizer / ListboxVirtualizer renders correctly when slot contains comment nodes
  • Verify normal virtualizer usage without comments still works

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed virtualized listbox rendering in certain component layouts.

When comment or text nodes are present in the virtualizer slot (e.g.
`<!-- @vue-expect-error -->`), the first Fragment child may not be the
actual component VNode. Filter by `typeof type !== 'symbol'` to skip
Comment, Text, Static, and Fragment VNodes.

Closes #2563

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Updated the virtualized slot's node selection logic to skip comment nodes and other non-element children, selecting the first valid child node instead of always using the first array element for virtualization metadata application.

Changes

Cohort / File(s) Summary
ListboxVirtualizer VNode Selection
packages/core/src/Listbox/ListboxVirtualizer.vue
Modified child node selection to find the first non-symbol child instead of always taking children[0], properly handling comment nodes in slot content.

Estimated Code Review Effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 When comments hide within the slot,
We skip the symbols, take them not,
The first real child we seek and find,
Virtualizing with proper mind! 🎯

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: ignoring non-element VNodes (comments, text, etc.) when selecting slot children in ListboxVirtualizer.
Linked Issues check ✅ Passed The code changes directly address issue #2563 by implementing the expected behavior of ignoring comment/text/static VNodes when locating the element VNode in slot content.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the ListboxVirtualizer slot rendering issue described in #2563; no out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@pkg-pr-new

pkg-pr-new Bot commented Apr 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/reka-ui@2578

commit: 0fdcee2

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/core/src/Listbox/ListboxVirtualizer.vue (1)

85-91: ⚠️ Potential issue | 🟡 Minor

Add a safe fallback when .find() returns no matching child.

Line 86 can return undefined when Fragment children are all symbol-typed nodes, which will break cloneVNode on line 91. Add a fallback to guarantee a valid VNode is always selected.

Suggested fix
-import { cloneVNode, computed, Fragment, useSlots } from 'vue'
+import { cloneVNode, computed, Fragment, isVNode, useSlots } from 'vue'
@@
-  const targetNode = defaultNode.type === Fragment && Array.isArray(defaultNode.children)
-    ? defaultNode.children.find(child => typeof (child as VNode).type !== 'symbol') as VNode
-    : defaultNode
+  const targetNode = defaultNode.type === Fragment && Array.isArray(defaultNode.children)
+    ? (defaultNode.children.find(
+        child => isVNode(child) && typeof child.type !== 'symbol',
+      ) as VNode | undefined) ?? defaultNode
+    : defaultNode

Consider adding a regression test for a Fragment slot containing only filtered nodes before the actual option node.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/Listbox/ListboxVirtualizer.vue` around lines 85 - 91, The
targetNode resolution can be undefined when Fragment children are all
symbol-typed; update the logic that sets targetNode (based on defaultNode,
Fragment and .find) to use a safe fallback (e.g., fall back to defaultNode or a
minimal VNode) before calling cloneVNode, so cloneVNode always receives a valid
VNode; update the code path that declares targetNode and ensure
cloneVNode(targetNode, {...}) is guarded accordingly and add a regression test
that renders a Fragment slot containing only filtered (symbol) nodes to verify
the fallback behavior.
🧹 Nitpick comments (1)
packages/core/src/Listbox/ListboxVirtualizer.vue (1)

85-87: Consider extracting VNode-selection into a shared helper for virtualizers.

ListboxVirtualizer now has robust selection logic, while packages/core/src/Tree/TreeVirtualizer.vue:93-111 still uses direct [0] access. A shared helper would keep behavior consistent and prevent divergence.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/Listbox/ListboxVirtualizer.vue` around lines 85 - 87,
Extract the VNode-selection logic currently in ListboxVirtualizer (the code that
computes targetNode from defaultNode using the Fragment check and child.find
where typeof child.type !== 'symbol') into a shared helper function (e.g.,
getPrimaryVNode or selectVisibleVNode) in a common utilities module used by
virtualizers; then replace the inline logic in ListboxVirtualizer and change
TreeVirtualizer (which currently uses direct [0] access) to call this helper so
both use the same robust selection behavior and avoid divergence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/core/src/Listbox/ListboxVirtualizer.vue`:
- Around line 85-91: The targetNode resolution can be undefined when Fragment
children are all symbol-typed; update the logic that sets targetNode (based on
defaultNode, Fragment and .find) to use a safe fallback (e.g., fall back to
defaultNode or a minimal VNode) before calling cloneVNode, so cloneVNode always
receives a valid VNode; update the code path that declares targetNode and ensure
cloneVNode(targetNode, {...}) is guarded accordingly and add a regression test
that renders a Fragment slot containing only filtered (symbol) nodes to verify
the fallback behavior.

---

Nitpick comments:
In `@packages/core/src/Listbox/ListboxVirtualizer.vue`:
- Around line 85-87: Extract the VNode-selection logic currently in
ListboxVirtualizer (the code that computes targetNode from defaultNode using the
Fragment check and child.find where typeof child.type !== 'symbol') into a
shared helper function (e.g., getPrimaryVNode or selectVisibleVNode) in a common
utilities module used by virtualizers; then replace the inline logic in
ListboxVirtualizer and change TreeVirtualizer (which currently uses direct [0]
access) to call this helper so both use the same robust selection behavior and
avoid divergence.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 82500c49-ab0f-4b82-a9ea-b85d60a3fc3a

📥 Commits

Reviewing files that changed from the base of the PR and between c04bed3 and 0fdcee2.

📒 Files selected for processing (1)
  • packages/core/src/Listbox/ListboxVirtualizer.vue

@zernonia zernonia merged commit 06042b4 into unovue:v2 Apr 6, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: <ListboxVirtualizer> slot should ignore comment nodes

1 participant