[solid] `asChild` prop issues after component updates

[arekmaz]

Description

in my fork of the repo I was trying to provide a fix for #2007, I wrote the following test: (in the solid spread.test.ts file)

during the initial render the parent class is correctly merged into the child, however after updates (clicks), it looks like the parent class is no longer present

I tried to investigate how is the asChild implemented, I know that the asChild topic for solid is a bit more tricky than e.g. in react, but I'm too new to solid

is this behavior expected but perhaps uncocumented, or is there a special way of using the asChild prop with solid, or is this just a bug and otherwise the asChild api is figured out?

Link to Reproduction (or Detailed Explanation)

https://codesandbox.io/p/devbox/blissful-butterfly-mkysr3?file=%2Fsrc%2FApp.tsx

Steps to Reproduce

1. go to repro link
2. click on the "bounce" button

Ark UI Version

1.2.0

Framework

• [ ] React
• [X] Solid
• [ ] Vue

Browser

ARC (Chromium)

Additional Information

No response

[cschroeter]

@arekmaz

I started to look into this issue for ohter reasons (hydration missmatch). This spread.test.ts is a bit strange. I started to mimic the React factory.test.tsx.

Here is what I have so far:

import { render, screen } from '@solidjs/testing-library'
import user from '@testing-library/user-event'
import { vi } from 'vitest'
import { ark } from './factory'

const ComponentUnderTest = () => (
  <ark.div
    id="parent"
    data-part="parent"
    data-testid="parent"
    class="parent"
    style={{ background: 'red' }}
    asChild
  >
    <ark.span
      id="child"
      data-part="child"
      data-testid="child"
      class="child"
      style={{ color: 'blue' }}
    >
      Ark UI
    </ark.span>
  </ark.div>
)

describe('Ark Factory', () => {
  it('should render only the child', () => {
    render(() => <ComponentUnderTest />)

    expect(() => screen.getByTestId('parent')).toThrow()
    expect(screen.getByTestId('child')).toBeVisible()
  })

  it('should override existing props', () => {
    render(() => <ComponentUnderTest />)
    const child = screen.getByTestId('child')
    expect(child.id).toBe('child')
    expect(child.dataset.part).toBe('child')
  })

  it('should merge styles and 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()
  })

  it('should merge events', async () => {
    const onClickParent = vi.fn()
    const onClickChild = vi.fn()
    render(() => (
      <ark.div data-testid="parent" onClick={onClickParent} asChild>
        <ark.span data-testid="child" onClick={onClickChild} />
      </ark.div>
    ))
    await user.click(screen.getByTestId('child'))
    expect(onClickParent).toHaveBeenCalled()
    expect(onClickChild).toHaveBeenCalled()
  })

  it('should propagate asChild', async () => {
    render(() => (
      <ark.div data-testid="parent" asChild>
        <ark.span asChild>
          <ark.span>Ark UI</ark.span>
        </ark.span>
      </ark.div>
    ))
    expect(screen.getByText('Ark UI')).toHaveAttribute('data-testid', 'parent')
  })

  it('should stop propagate asChild', async () => {
    render(() => (
      <ark.div data-testid="parent" asChild>
        <ark.span asChild={false}>
          <ark.span>Ark UI</ark.span>
        </ark.span>
      </ark.div>
    ))
    expect(screen.getByText('Ark UI')).not.toHaveAttribute('data-testid', 'parent')
  })

  it('should stop propagate asChild', async () => {
    render(() => (
      <ark.div data-testid="parent" asChild>
        <ark.span asChild={false}>
          <ark.span>Ark UI</ark.span>
        </ark.span>
      </ark.div>
    ))
    expect(screen.getByText('Ark UI')).not.toHaveAttribute('data-testid', 'parent')
  })
})

[arekmaz]

I think this asChild functionality is not so simple like in react, there is no children.props, only the children dom node,

probably that's why e.g. kobalte has this As component, to merge props

https://kobalte.dev/docs/core/overview/polymorphism

[arekmaz]

@cschroeter I fixed your svg test in https://github.com/chakra-ui/ark/pull/2009/files, but, again, that's only for initial render, updates are still broken for every component

[arekmaz]

I created https://github.com/chakra-ui/ark/pull/2010 as a possible way to solve this

[cschroeter]

@arekmaz

First of all, let me say how amazing it is that you already have a working solution in place and that you spend the extra effort to create this PR.

I've discussed this with @segunadebayo in our meeting today, and we would prefer an API for non-virtual DOM-based frameworks like Solid or Svelte that looks something like this:

<Popover.Trigger as={Button}>
  Hello
</Popover.Trigger>

This already works but requires a bit of TypeScript gymnastics to make it work eventually.

// factory.tsx
function withAsProp(Component: ElementType) {
  return function jsx(props: ParentProps<AsChildProps>) {
    const [localProps, restProps] = splitProps(props, ['as', 'children'])

    return (
      <Dynamic component={localProps.as || Component} {...restProps}>
        {localProps.children}
      </Dynamic>
    )
  }
}

[arekmaz]

I think the asChild approach is more powerful and flexible, I like the as prop from chakra but it felt always a bit limited for me, when I saw the asChild pattern I started to imagine some crazy composition scenarios, e.g.

<TextItalic asChild>
  <As component={ButtonRed} asChild>
    <As component={Select.Trigger}>
      <SelectIcon />
    </As>
   </As>
</TextItalic>

I'm not saying that's a good pattern but at least something like this would be possible, with the as prop the composition is not nestable,

would you consider maybe supporting both? I think they are quite a bit different mechanisms anyway....

besides with the as prop, you can only provide the child props, there is nothing to be merged, you just pass the props down,

[asterikx]

I have this "update issue" with a lot of Park-UI components. For example, for controlled number inputs that, for some reason, become non-interactive if the value prop is undefined during the first render. I wonder if the splitProps implementation of Panda CSS is compatible with Solid (and similarly mergeProps). If not, I could imagine that prop reactivity is lost there. I already tried to debug, but couldn't create a repro so far due to inconsistent behaviors. I acknowledge, that my issue might be another issue, but it could be the root cause for both issues.

[cschroeter]

@asterikx

I agree. I also think that our current asChild has some limitations for non virtual dom frameworks. However the with the "new" approach <NumberInput.Input as={Input}> everything works as expected.

What's next? At the moment I try to figure out what limitations we could ran into using the as prop vs asChild + <As />

[arekmaz]

@cschroeter

great to hear, btw this issue blocks me a bit on my https://arek-ui-solid.fly.dev/ components, I would be happy to help if I can

[asterikx]

@cschroeter in case the splitProps and mergeProps implementations in Panda CSS are the root of these problems, what speaks against fixing the problem right there in Panda? I would picture Panda simply using splitProps and mergeProps from solid-js when jsxFramework is set to solid in Panda instead of its own implementations (solid-js would become a peer dependency of Panda). I'm not particularly fan of the as prop in general, as it obfuscates and complicates the component API (e.g. resulting prop types, even worse with generic components, like Combobox) and a lot of internal things. I think Segun also mentioned in his blog post about the future of Chakra UI that the as prop has added a lot of complexity to the codebase of Chakra UI and is a maintenance bottleneck. Re-introducing the as prop feels like a workaround to an underlying issue and possibly the wrong move in terms of component API and codebase maintainability? But of course that's just my personal opinion and I have no detailed knowledge about the "new" as prop. Regarding the update problem, it is of course possible that the problem also lies with Ark or Zag and not with Panda. I'm still a bit in the dark and can't reproduce it reliably. I would like to use this comment to thank you for your tireless efforts at Park, Ark and Panda @cschroeter. I truly believe that it is currently the best way to efficiently build stunning, type-safe, performant, and (mostly) framework-agnostic frontends and I enjoy it every day. This would certainly not be the case without your commitment. Wherever I can, I will be happy to support. 🚀

[cschroeter]

This will be resolved in the next major release of Ark UI Solid