Skip to content

Commit fc8900e

Browse files
committed
fix(vue): correct prop merging in the asChild factory
`Dynamic` merged the parent attrs with the child's props, then handed the result to `cloneVNode` without clearing the child's own props. `cloneVNode` merges again, so the child's `class` landed twice (`class="child parent child"`) and its event handlers were re-added alongside the composed ones. Cloning from a vnode with cleared props makes the second merge a no-op, which also removes the loop that existed to undo the handler half of the same problem. `Dynamic` also took `children[0]` blindly. A leading comment (a plain comment or the placeholder a false `v-if` leaves behind) meant the props were applied to the comment and silently dropped. It now targets the first non-comment child and replaces it in place, so the other children keep their order. Class merge order now matches React (`parent child`). Docs: the composition guide claimed the factory example rendered a `span` with props that appear in no example. It renders `<a href="#">Ark UI</a>` in React, Solid, and Vue alike. Also switched the Vue example to `as-child`, the dominant spelling in this package's examples.
1 parent 485d174 commit fc8900e

5 files changed

Lines changed: 95 additions & 14 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@ark-ui/vue": patch
3+
---
4+
5+
- Fixed `asChild` duplicating the child element's `class`, so `<ark.div class="parent" as-child><span class="child">`
6+
rendered `class="child parent child"` instead of `class="parent child"`.
7+
- Fixed `asChild` applying props to a leading comment node, which silently dropped them when a comment or a false `v-if`
8+
preceded the child element.

packages/vue/src/components/factory.test.tsx

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import user from '@testing-library/user-event'
22
import { render, screen } from '@testing-library/vue'
3+
import { createCommentVNode, defineComponent, h, nextTick, ref } from 'vue'
34
import { ark } from './factory.ts'
45

56
const ComponentUnderTest = (
@@ -33,6 +34,75 @@ describe('Factory', () => {
3334
expect(screen.getByText('Ark UI')).toBeVisible()
3435
})
3536

37+
it('should not duplicate the class of a plain child element', () => {
38+
render(
39+
<ark.div class="parent" asChild>
40+
<span data-testid="child" class="child">
41+
Ark UI
42+
</span>
43+
</ark.div>,
44+
)
45+
const child = screen.getByTestId('child')
46+
expect(child.className.split(/\s+/).filter(Boolean).sort()).toEqual(['child', 'parent'])
47+
})
48+
49+
it('should call each handler of a plain child element once', async () => {
50+
const onClickParent = vi.fn()
51+
const onClickChild = vi.fn()
52+
render(
53+
<ark.div onClick={onClickParent} asChild>
54+
<button type="button" data-testid="child" onClick={onClickChild} />
55+
</ark.div>,
56+
)
57+
await user.click(screen.getByTestId('child'))
58+
expect(onClickParent).toHaveBeenCalledTimes(1)
59+
expect(onClickChild).toHaveBeenCalledTimes(1)
60+
})
61+
62+
it('should apply props to the first non-comment child', () => {
63+
render(
64+
defineComponent({
65+
setup: () => () =>
66+
h(ark.div, { class: 'parent', asChild: true }, () => [
67+
createCommentVNode('placeholder'),
68+
h('span', { 'data-testid': 'child', class: 'child' }, 'Ark UI'),
69+
]),
70+
}),
71+
)
72+
const child = screen.getByTestId('child')
73+
expect(child.className.split(/\s+/).filter(Boolean).sort()).toEqual(['child', 'parent'])
74+
})
75+
76+
it('should patch reactive props onto the same child element', async () => {
77+
const parentClass = ref('a')
78+
const { container } = render(
79+
defineComponent({
80+
setup: () => () =>
81+
h(ark.div, { class: parentClass.value, asChild: true }, () => [
82+
h('span', { 'data-testid': 'child', class: 'child' }, 'Ark UI'),
83+
]),
84+
}),
85+
)
86+
const before = screen.getByTestId('child')
87+
expect(before.className.split(/\s+/).filter(Boolean).sort()).toEqual(['a', 'child'])
88+
89+
parentClass.value = 'b'
90+
await nextTick()
91+
92+
const after = container.querySelector('[data-testid="child"]')
93+
expect(after).toBe(before)
94+
expect(after?.className.split(/\s+/).filter(Boolean).sort()).toEqual(['b', 'child'])
95+
})
96+
97+
it('should render comment-only children untouched', () => {
98+
const { container } = render(
99+
defineComponent({
100+
setup: () => () => h(ark.div, { class: 'parent', asChild: true }, () => [createCommentVNode('v-if')]),
101+
}),
102+
)
103+
expect(container.innerHTML).toBe('<!--v-if-->')
104+
})
105+
36106
it('should merge events', async () => {
37107
const onClickParent = vi.fn()
38108
const onClickChild = vi.fn()

packages/vue/src/components/popover/examples/factory.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { ark } from '@ark-ui/vue/factory'
33
</script>
44

55
<template>
6-
<ark.span asChild>
6+
<ark.span as-child>
77
<a href="#">Ark UI</a>
88
</ark.span>
99
</template>

packages/vue/src/utils/dynamic.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { mergeProps } from '@zag-js/vue'
2-
import { Fragment, type VNode, cloneVNode, defineComponent } from 'vue'
2+
import { Comment, Fragment, type VNode, cloneVNode, defineComponent } from 'vue'
33

44
export const Dynamic = defineComponent({
55
name: 'Dynamic',
@@ -8,20 +8,20 @@ export const Dynamic = defineComponent({
88
return () => {
99
if (!slots.default) return null
1010
const children = renderSlotFragments(slots.default())
11-
const [firstChildren, ...otherChildren] = children
11+
const index = children.findIndex((child) => child.type !== Comment)
12+
if (index === -1) return children
1213

13-
if (firstChildren && Object.keys(attrs).length > 0) {
14+
const firstChildren = children[index]
15+
16+
if (Object.keys(attrs).length > 0) {
1417
delete firstChildren.props?.ref
18+
// props are cleared below so `cloneVNode` doesn't merge the child's own props a second time
1519
const mergedProps = mergeProps(attrs, firstChildren.props ?? {})
16-
const cloned = cloneVNode(firstChildren, mergedProps)
17-
for (const prop in mergedProps) {
18-
if (prop.startsWith('on')) {
19-
cloned.props ||= {}
20-
cloned.props[prop] = mergedProps[prop]
21-
}
22-
}
20+
const cloned = cloneVNode({ ...firstChildren, props: {} }, mergedProps)
2321

24-
return children.length === 1 ? cloned : [cloned, ...otherChildren]
22+
if (children.length === 1) return cloned
23+
children[index] = cloned
24+
return children
2525
}
2626

2727
return children

website/src/content/pages/guides/composition.mdx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,15 @@ You can use the `ark` factory to create your own elements that work just like Ar
2222

2323
<ExampleCode id="factory" component="popover" />
2424

25-
This will produce the following HTML:
25+
The factory renders the child element in place of the `span`, so this produces:
2626

2727
```html
28-
<span id="child" class="parent child" style="background: red; color: blue;">Ark UI</span>
28+
<a href="#">Ark UI</a>
2929
```
3030

31+
Any props you pass to `ark.span` are merged onto the child element, which is how the factory forwards styling and behavior to
32+
whatever you render.
33+
3134
## ID Composition
3235

3336
When composing components that need to work together, share IDs between them using the `ids` prop for proper

0 commit comments

Comments
 (0)