Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.8k
feat(runtime-core): add unwrapFragment to flatten nested Fragment nodes in vnode arrays#13546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
coderabbitaibot commentedJul 1, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 arrayPoem
✨ 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 withCodeRabbit:
SupportNeed help? Create a ticket on oursupport 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 ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fix unsafe type assertion.
The type assertionvnode.children as VNode[] is unsafe. Fragment children can be of typeVNodeNormalizedChildren which includesstring | 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 unsafelyasserts vnode.children as VNode[], which can cause runtime errors since childrenmay be other types like string or null. Fix this by adding a type check toconfirm vnode.children is an array before spreading and recursing, ensuringunwrapFragment 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. |
I've opened a corresponding RFC for |
Uh oh!
There was an error while loading.Please reload this page.
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 tomultiple levels of nested Fragment nodes.This nesting creates practical issues in real-world applications, especially in component libraries. For example, inTencent'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 toflatten 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