feat(solid): revise asChild factory

chakra-ui · May 17, 2024 · b186861 · b186861
1 parent 0d8c4f0
commit b186861
Show file tree

Hide file tree

Showing 39 changed files with 1,072 additions and 331 deletions.
diff --git a/packages/solid/src/components/checkbox/checkbox-hidden-input.tsx b/packages/solid/src/components/checkbox/checkbox-hidden-input.tsx
@@ -2,7 +2,7 @@ import { mergeProps } from '@zag-js/solid'
 import { type HTMLArkProps, ark } from '../factory'
 import { useCheckboxContext } from './use-checkbox-context'
 
-export interface CheckboxHiddenInputProps extends HTMLArkProps<'div'> {}
+export interface CheckboxHiddenInputProps extends HTMLArkProps<'input'> {}
 
 export const CheckboxHiddenInput = (props: CheckboxHiddenInputProps) => {
   const checkbox = useCheckboxContext()

diff --git a/packages/solid/src/components/clipboard/clipboard-indicator.tsx b/packages/solid/src/components/clipboard/clipboard-indicator.tsx
@@ -15,7 +15,6 @@ export const ClipboardIndicator = (props: ClipboardIndicatorProps) => {
   const [indicatorProps, localProps] = createSplitProps<IndicatorProps>()(props, ['copied'])
   const api = useClipboardContext()
   const mergedProps = mergeProps(api().getIndicatorProps({ copied: api().copied }), localProps)
-  // @ts-expect-error TODO fix
   const getChildren = children(() => localProps.children)
 
   return (

diff --git a/packages/solid/src/components/factory.test.tsx b/packages/solid/src/components/factory.test.tsx
@@ -10,84 +10,66 @@ const ComponentUnderTest = () => (
     data-testid="parent"
     class="parent"
     style={{ background: 'red' }}
-    asChild
-  >
-    {(props) => (
+    asChild={(props) => (
       <ark.span
         {...props({ id: 'child', class: 'child', style: { color: 'blue' } })}
         data-part="child"
         data-testid="child"
       >
-        Ark UI
+        Child
       </ark.span>
     )}
+  >
+    Parent
   </ark.div>
 )
 
 describe('Ark Factory', () => {
   it('should render only the child', () => {
     render(() => <ComponentUnderTest />)
-
-    expect(() => screen.getByTestId('parent')).toThrow()
-    expect(screen.getByTestId('child')).toBeVisible()
+    expect(screen.getByText('Child')).toBeVisible()
   })
 
-  it('should override existing props', () => {
+  it('should merge styles', () => {
     render(() => <ComponentUnderTest />)
-    const child = screen.getByTestId('child')
-    expect(child.id).toBe('child')
-    // biome-ignore lint/complexity/useLiteralKeys: <explanation>
-    expect(child.dataset['part']).toBe('child')
+    expect(screen.getByText('Child')).toHaveStyle({ color: 'rgb(0, 0, 255)', background: 'red' })
   })
 
-  it('should merge styles and classes', () => {
+  it('should merge classes', () => {
     render(() => <ComponentUnderTest />)
-    const child = screen.getByTestId('child')
-    expect(child).toHaveStyle({ background: 'red' })
-    expect(child).toHaveStyle({ color: 'rgb(0, 0, 255)' })
-    expect(child).toHaveClass('child parent')
-    expect(screen.getByText('Ark UI')).toBeVisible()
+    expect(screen.getByText('Child')).toHaveClass('child parent')
   })
 
-  // TODO: Fix this test
   it('should merge events', async () => {
     const onClickParent = vi.fn()
     const onClickChild = vi.fn()
     render(() => (
-      <ark.div data-testid="parent" onClick={onClickParent} asChild>
-        {(props) => <ark.span {...props({ onClick: onClickChild })} data-testid="child" />}
+      <ark.div
+        data-testid="parent"
+        onClick={onClickParent}
+        asChild={(props) => <ark.span {...props({ onClick: onClickChild })} data-testid="child" />}
+      >
+        Parent
       </ark.div>
     ))
     await user.click(screen.getByTestId('child'))
     expect(onClickParent).toHaveBeenCalled()
     expect(onClickChild).toHaveBeenCalled()
   })
 
-  it.skip('should propagate asChild', async () => {
+  it('should stop propagate asChild', async () => {
     render(() => (
-      <ark.div data-testid="parent" asChild>
-        {(props) => (
+      <ark.div
+        data-testid="parent"
+        asChild={(props) => (
           <ark.span {...props()}>
-            <ark.span>Ark UI</ark.span>
+            <ark.span>Child</ark.span>
           </ark.span>
         )}
+      >
+        Parent
       </ark.div>
     ))
-    expect(screen.getByText('Ark UI')).toHaveAttribute('data-testid', 'parent')
-  })
-
-  it('should stop propagate asChild', async () => {
-    render(() => (
-      <p>
-        <ark.div data-testid="parent" asChild>
-          {(props) => (
-            <ark.span {...props()}>
-              <ark.span>Ark UI</ark.span>
-            </ark.span>
-          )}
-        </ark.div>
-      </p>
-    ))
-    expect(screen.getByText('Ark UI')).not.toHaveAttribute('data-testid', 'parent')
+    expect(screen.getByText('Child')).not.toHaveAttribute('data-testid', 'parent')
   })
 })
diff --git a/packages/solid/src/components/factory.tsx b/packages/solid/src/components/factory.tsx
@@ -9,13 +9,13 @@ type JsxElements = {
   [E in ElementType]: ArkComponent<E>
 }
 
-type PropsFn<T extends ElementType> = (
+type ParentProps<T extends ElementType> = (
   userProps?: JSX.IntrinsicElements[T],
-) => JSX.HTMLAttributes<HTMLElement>
+  // biome-ignore lint/suspicious/noExplicitAny: <explanation>
+) => JSX.HTMLAttributes<any>
 
 type PolymorphicProps<T extends ElementType> = {
-  asChild?: boolean
-  children?: JSX.Element | ((props: PropsFn<T>) => JSX.Element)
+  asChild?: (props: ParentProps<T>) => JSX.Element
 }
 
 type HTMLArkProps<E extends ElementType> = Assign<ComponentProps<E>, PolymorphicProps<E>>
@@ -24,26 +24,20 @@ type ArkComponent<E extends ElementType> = (props: HTMLArkProps<E>) => JSX.Eleme
 
 const withAsProp = <T extends ElementType>(Component: T) => {
   const ArkComponent: ArkComponent<T> = (props) => {
-    const [localProps, otherProps] = splitProps(props, ['asChild'])
+    const [localProps, parentProps] = splitProps(props, ['asChild'])
 
     if (localProps.asChild) {
-      if (typeof otherProps.children !== 'function') {
-        throw new Error('Children must be a function')
+      // @ts-expect-error
+      const propsFn = (userProps) => {
+        const [, restProps] = splitProps(parentProps, ['ref'])
+        return { ref: parentProps.ref, ...mergeProps(restProps, userProps) }
       }
-
-      // @ts-expect-error TODO improve
-      const fn = (userProps) => {
-        const [, restProps] = splitProps(otherProps, ['children', 'ref'])
-
-        return { ref: otherProps.ref, ...mergeProps(restProps, userProps) }
-      }
-
-      return <>{otherProps.children(fn)}</>
+      return localProps.asChild(propsFn)
     }
-
-    // biome-ignore lint/suspicious/noExplicitAny: <explanation>
-    return <Dynamic component={Component} {...(otherProps as any)} />
+    // @ts-expect-error
+    return <Dynamic component={Component} {...parentProps} />
   }
+
   return ArkComponent
 }
 

diff --git a/packages/solid/src/components/menu/examples/radio-group.tsx b/packages/solid/src/components/menu/examples/radio-group.tsx
@@ -15,7 +15,7 @@ export const RadioGroup = () => {
               {(framework) => (
                 <Menu.RadioItem value={framework()} disabled={framework() === 'Svelte'}>
                   <Menu.ItemIndicator>✅</Menu.ItemIndicator>
-                  <Menu.ItemText>{framework}</Menu.ItemText>
+                  <Menu.ItemText>{framework()}</Menu.ItemText>
                 </Menu.RadioItem>
               )}
             </Index>

diff --git a/website/src/content/pages/guides/styling.mdx b/website/src/content/pages/guides/styling.mdx
@@ -53,7 +53,7 @@ To style a component based on its state, use the `data-state` attribute:
 > **Tip:** If you prefer using classes instead of data attributes, utilize the `class` or
 > `className` prop to add custom classes to Ark UI components.
 
-## Styling with Panda
+## Styling with Panda CSS
 
 [Panda CSS](https://panda-css.com) is a build-time CSS-in-JS framework that integrates seamlessly
 with Ark UI, providing an efficient styling solution.

diff --git a/website/src/content/types/solid/accordion.types.json b/website/src/content/types/solid/accordion.types.json
@@ -1,25 +1,37 @@
 {
-  "ItemContent": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
-  "ItemIndicator": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
+  "ItemContent": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false }
+    }
+  },
+  "ItemIndicator": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false }
+    }
+  },
   "Item": {
     "props": {
       "value": {
         "type": "string",
         "isRequired": true,
         "description": "The value of the accordion item."
       },
-      "asChild": { "type": "boolean", "isRequired": false },
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false },
       "disabled": {
         "type": "boolean",
         "isRequired": false,
         "description": "Whether the accordion item is disabled."
       }
     }
   },
-  "ItemTrigger": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
+  "ItemTrigger": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'button'>) => Element", "isRequired": false }
+    }
+  },
   "Root": {
     "props": {
-      "asChild": { "type": "boolean", "isRequired": false },
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false },
       "collapsible": {
         "type": "boolean",
         "defaultValue": "false",

diff --git a/website/src/content/types/solid/avatar.types.json b/website/src/content/types/solid/avatar.types.json
@@ -1,9 +1,17 @@
 {
-  "Fallback": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
-  "Image": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
+  "Fallback": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'span'>) => Element", "isRequired": false }
+    }
+  },
+  "Image": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'img'>) => Element", "isRequired": false }
+    }
+  },
   "Root": {
     "props": {
-      "asChild": { "type": "boolean", "isRequired": false },
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false },
       "id": {
         "type": "string",
         "isRequired": false,

diff --git a/website/src/content/types/solid/carousel.types.json b/website/src/content/types/solid/carousel.types.json
@@ -1,22 +1,42 @@
 {
-  "Control": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
-  "IndicatorGroup": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
+  "Control": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false }
+    }
+  },
+  "IndicatorGroup": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false }
+    }
+  },
   "Indicator": {
     "props": {
       "index": { "type": "number", "isRequired": true },
-      "asChild": { "type": "boolean", "isRequired": false },
+      "asChild": { "type": "(props: ParentProps<'button'>) => Element", "isRequired": false },
       "readOnly": { "type": "boolean", "isRequired": false }
     }
   },
-  "ItemGroup": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
+  "ItemGroup": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false }
+    }
+  },
   "Item": {
     "props": {
       "index": { "type": "number", "isRequired": true, "description": "The index of the item." },
-      "asChild": { "type": "boolean", "isRequired": false }
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false }
+    }
+  },
+  "NextTrigger": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'button'>) => Element", "isRequired": false }
+    }
+  },
+  "PrevTrigger": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'button'>) => Element", "isRequired": false }
     }
   },
-  "NextTrigger": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
-  "PrevTrigger": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
   "Root": {
     "props": {
       "align": {
@@ -25,7 +45,7 @@
         "isRequired": false,
         "description": "The alignment of the slides in the carousel."
       },
-      "asChild": { "type": "boolean", "isRequired": false },
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false },
       "defaultIndex": {
         "type": "number",
         "isRequired": false,
@@ -73,5 +93,9 @@
       }
     }
   },
-  "Viewport": { "props": { "asChild": { "type": "boolean", "isRequired": false } } }
+  "Viewport": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false }
+    }
+  }
 }
diff --git a/website/src/content/types/solid/checkbox.types.json b/website/src/content/types/solid/checkbox.types.json
@@ -1,16 +1,28 @@
 {
-  "Control": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
-  "HiddenInput": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
+  "Control": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false }
+    }
+  },
+  "HiddenInput": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'input'>) => Element", "isRequired": false }
+    }
+  },
   "Indicator": {
     "props": {
-      "asChild": { "type": "boolean", "isRequired": false },
+      "asChild": { "type": "(props: ParentProps<'div'>) => Element", "isRequired": false },
       "indeterminate": { "type": "boolean", "isRequired": false }
     }
   },
-  "Label": { "props": { "asChild": { "type": "boolean", "isRequired": false } } },
+  "Label": {
+    "props": {
+      "asChild": { "type": "(props: ParentProps<'span'>) => Element", "isRequired": false }
+    }
+  },
   "Root": {
     "props": {
-      "asChild": { "type": "boolean", "isRequired": false },
+      "asChild": { "type": "(props: ParentProps<'label'>) => Element", "isRequired": false },
       "checked": {
         "type": "CheckedState",
         "isRequired": false,