Simplify complex asChild scenarios and fix reconciliation issues

[benoitgrelard]

Description

This PR replaces the getRoot helper with a new helper that is simpler and doesn't have the caveats of creating a new component on each render. This should fix the issues observed by @lucasmotta.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
themes-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 21, 2024 11:39am

[benoitgrelard]

I went with the render prop approach as I felt it was actually the simplest to understand the overall abstraction in one go whilst still retaining good ergonomics looking at the tree and colocation of the inner content.

The alternative I tested was to have 2 functions:

• one placed where firstChildMightAdoptSubtree currently is but taking a "node" as second argument…
• …in which you'd call a second like getConsumerChildren({ asChild, children }) inline in your nested DOM

Would look something like this for this component:

<NavigationMenu.Link
  {...linkProps}
  ref={forwardedRef}
  className={classNames('rt-reset', 'rt-BaseTabListTrigger', 'rt-TabNavLink', className)}
  onSelect={() => {}}
  asChild={asChild}
>
  {firstChildMightAdoptSubtree({ asChild, children },
    <>
      <span className="rt-BaseTabListTriggerInner rt-TabNavLinkInner">
        {getConsumerChildren({ asChild, children })}
      </span>
      <span className="rt-BaseTabListTriggerInnerHidden rt-TabNavLinkInnerHidden">
        {getConsumerChildren({ asChild, children })}
      </span>
    </>
  )}
</NavigationMenu.Link>

Note that this case needs 2 instances of the children so when I had that, I actually prefered abstracting the inner stuff in its own component so getConsumerChildren was only called once.

Overall, I think the render prop does a better job but let me know what you think!

[benoitgrelard]

Added the simple case where we don't need children as convenience (for cases like Avatar where the component actually doesn't take children (other than for asChild purposes).