feat(runtime-core): add unwrapFragment to flatten nested Fragment nodes in vnode arrays #13546
Conversation
…es in vnode arrays
WalkthroughA new utility function, Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant unwrapFragment
participant isFragmentVNode
Caller->>unwrapFragment: unwrapFragment(vnodes)
alt vnodes is undefined or empty
unwrapFragment-->>Caller: []
else For each vnode in vnodes
unwrapFragment->>isFragmentVNode: isFragmentVNode(vnode)
alt vnode is fragment
unwrapFragment->>unwrapFragment: unwrapFragment(vnode.children)
unwrapFragment-->>unwrapFragment: ...flattened children
else vnode is not fragment
unwrapFragment-->>unwrapFragment: vnode
end
end
unwrapFragment-->>Caller: flattened array
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/runtime-core/src/unwrapFragment.ts (1)
4-5: Use English comments for consistency.The codebase uses English comments throughout. Please translate the Chinese comment to English for consistency.
-/** - * 展开 vnode 数组中所有 Fragment 节点，将它们的子节点平铺出来。 - */ +/** + * Recursively flatten all Fragment nodes in a vnode array, expanding their children. + */packages/runtime-core/__tests__/unwrapFragment.spec.ts (1)
4-48: Comprehensive test suite with good coverage.The test suite covers the main use cases well and follows proper testing patterns. Consider adding a test case for fragments with non-array children to ensure the function handles edge cases gracefully.
Consider adding this test case to cover edge cases:
+ it('handles Fragment with non-array children gracefully', () => { + // Create a fragment with string children (edge case) + const fragmentVNode: VNode = h(Fragment, null, 'text content') + const input = [fragmentVNode] + const result = unwrapFragment(input) + // Should not crash and handle gracefully + expect(result).toEqual([fragmentVNode]) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/runtime-core/__tests__/unwrapFragment.spec.ts(1 hunks)packages/runtime-core/src/index.ts(1 hunks)packages/runtime-core/src/unwrapFragment.ts(1 hunks)packages/runtime-core/src/vnode.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/runtime-core/src/vnode.ts (1)
packages/runtime-core/src/index.ts (2)
VNode(252-252)Fragment(113-113)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (2)
packages/runtime-core/src/vnode.ts (1)
390-392: LGTM! Well-implemented type guard function.The
isFragmentVNodefunction correctly implements a type guard following Vue's established patterns. It safely checks for a truthy vnode before accessing thetypeproperty and correctly compares against theFragmentsymbol.packages/runtime-core/src/index.ts (1)
128-128: Export placement looks correct.The
unwrapFragmentexport is appropriately positioned and follows Vue's export conventions.
| const result: VNode[] = [] | ||
| for (const vnode of vnodes) { | ||
| if (isFragmentVNode(vnode)) { | ||
| result.push(...unwrapFragment(vnode.children as VNode[])) |
There was a problem hiding this comment.
Fix unsafe type assertion.
The type assertion vnode.children as VNode[] is unsafe. Fragment children can be of type VNodeNormalizedChildren which includes string | VNodeArrayChildren | RawSlots | null. This could cause runtime errors if fragment children are not an array.
Apply this fix to handle the type safely:
- result.push(...unwrapFragment(vnode.children as VNode[]))
+ const children = vnode.children
+ if (Array.isArray(children)) {
+ result.push(...unwrapFragment(children))
+ }This ensures we only recurse when children is actually an array, preventing runtime errors.
🤖 Prompt for AI Agents
In packages/runtime-core/src/unwrapFragment.ts at line 11, the code unsafely
asserts vnode.children as VNode[], which can cause runtime errors since children
may be other types like string or null. Fix this by adding a type check to
confirm vnode.children is an array before spreading and recursing, ensuring
unwrapFragment is only called on arrays to prevent errors.
|
This is considered adding a new API and needs to go through the RFC process to gather feedback from community first. |
edison1105
added
🛑 on hold
Background
In Vue 3, when a slot returns multiple root nodes, Vue automatically wraps them in a
Fragment. When slot content is passed through multiple layers of components, this leads to multiple levels of nested Fragment nodes.This nesting creates practical issues in real-world applications, especially in component libraries. For example, in Tencent's open source component library TDesign, components like
Alertinternally inspect slot content to determine whether to display "expand/collapse" buttons based on vnode count. However, when these components are wrapped and slots are passed down, the slot content becomes wrapped in deeply nested Fragments, leading to incorrect logic.Vue currently does not provide an official utility to flatten such vnode structures.
This PR
This PR introduces a new internal helper:
unwrapFragment(vnodes: VNode[] | undefined): VNode[], which recursively flattens allFragmentnodes in a vnode array, exposing the "real" slot content.This improves:
Example Use Case