[pull] main from chakra-ui:main

.changeset/compose-refs-cleanup.md

@@ -0,0 +1,5 @@
+---
+"@ark-ui/react": patch
+---
+
+Fix `composeRefs`/`useComposedRefs` not resetting plain callback refs and object refs to `null` on detach when composed alongside a React 19 ref that returns a cleanup function.

packages/react/src/components/toggle-group/examples/with-tooltip.tsx

@@ -0,0 +1,37 @@
+import { Portal } from '@ark-ui/react/portal'
+import { ToggleGroup } from '@ark-ui/react/toggle-group'
+import { Tooltip, useTooltip } from '@ark-ui/react/tooltip'
+import { BoldIcon, ItalicIcon, UnderlineIcon } from 'lucide-react'
+import styles from 'styles/toggle-group.module.css'
+import tooltipStyles from 'styles/tooltip.module.css'
+
+const items = [
+  { value: 'bold', label: 'Bold', icon: <BoldIcon /> },
+  { value: 'italic', label: 'Italic', icon: <ItalicIcon /> },
+  { value: 'underline', label: 'Underline', icon: <UnderlineIcon /> },
+]
+
+const getTriggerId = (value?: string) => `toggle-item:${value}`
+
+export const WithTooltip = () => {
+  const tooltip = useTooltip({ ids: { trigger: getTriggerId } })
+
+  return (
+    <Tooltip.RootProvider value={tooltip}>
+      <ToggleGroup.Root defaultValue={['bold']} ids={{ item: getTriggerId }} className={styles.Root}>
+        {items.map((item) => (
+          <ToggleGroup.Item key={item.value} value={item.value} aria-label={item.label} className={styles.Item} asChild>
+            <Tooltip.Trigger value={item.value}>{item.icon}</Tooltip.Trigger>
+          </ToggleGroup.Item>
+        ))}
+      </ToggleGroup.Root>
+      <Portal>
+        <Tooltip.Positioner>
+          <Tooltip.Content className={tooltipStyles.Content}>
+            {items.find((item) => item.value === tooltip.triggerValue)?.label}
+          </Tooltip.Content>
+        </Tooltip.Positioner>
+      </Portal>
+    </Tooltip.RootProvider>
+  )
+}

packages/react/src/components/toggle-group/toggle-group.stories.tsx

@@ -10,3 +10,4 @@ export { Basic } from './examples/basic.tsx'
 export { Controlled } from './examples/controlled.tsx'
 export { Multiple } from './examples/multiple.tsx'
 export { RootProvider } from './examples/root-provider.tsx'
+export { WithTooltip } from './examples/with-tooltip.tsx'

packages/react/src/utils/compose-refs.test.tsx

@@ -1,6 +1,7 @@
 import { render, screen } from '@testing-library/react'
 import user from '@testing-library/user-event'
 import { useRef, useState } from 'react'
+import type { RefObject } from 'react'
 import { composeRefs, useComposedRefs } from './compose-refs.ts'
 
 describe('Util: composeRefs', () => {
@@ -25,6 +26,99 @@ describe('Util: composeRefs', () => {
 
     expect(cleanup).toHaveBeenCalledTimes(1)
   })
+
+  it('should not return a cleanup function when no ref provides one', () => {
+    const node = document.createElement('div')
+    const callbackRef = vi.fn()
+    const objectRef = { current: null as HTMLDivElement | null }
+
+    const dispose = composeRefs(callbackRef, objectRef)(node)
+
+    expect(dispose).toBeUndefined()
+  })
+
+  it('should reset a plain callback ref to null when a sibling ref returns a cleanup', () => {
+    const node = document.createElement('div')
+    const cleanup = vi.fn()
+    const refWithCleanup = vi.fn(() => cleanup)
+    const plainCallbackRef = vi.fn()
+
+    const dispose = composeRefs(refWithCleanup, plainCallbackRef)(node) as VoidFunction | undefined
+    expect(plainCallbackRef).toHaveBeenCalledWith(node)
+    expect(plainCallbackRef).not.toHaveBeenCalledWith(null)
+
+    dispose?.()
+
+    expect(cleanup).toHaveBeenCalledTimes(1)
+    expect(plainCallbackRef).toHaveBeenCalledWith(null)
+  })
+
+  it('should reset an object ref to null when a sibling ref returns a cleanup', () => {
+    const node = document.createElement('div')
+    const cleanup = vi.fn()
+    const refWithCleanup = vi.fn(() => cleanup)
+    const objectRef = { current: null as HTMLDivElement | null }
+
+    const dispose = composeRefs(refWithCleanup, objectRef)(node) as VoidFunction | undefined
+    expect(objectRef.current).toBe(node)
+
+    dispose?.()
+
+    expect(cleanup).toHaveBeenCalledTimes(1)
+    expect(objectRef.current).toBeNull()
+  })
+
+  it('should clean up every ref type when mixed together', () => {
+    const node = document.createElement('div')
+    const cleanupA = vi.fn()
+    const cleanupB = vi.fn()
+    const refWithCleanupA = vi.fn(() => cleanupA)
+    const refWithCleanupB = vi.fn(() => cleanupB)
+    const plainCallbackRef = vi.fn()
+    const objectRefA = { current: null as HTMLDivElement | null }
+    const objectRefB = { current: null as HTMLDivElement | null }
+
+    const dispose = composeRefs(refWithCleanupA, plainCallbackRef, objectRefA, refWithCleanupB, objectRefB)(node) as
+      VoidFunction | undefined
+
+    dispose?.()
+
+    expect(cleanupA).toHaveBeenCalledTimes(1)
+    expect(cleanupB).toHaveBeenCalledTimes(1)
+    expect(plainCallbackRef).toHaveBeenCalledWith(null)
+    expect(objectRefA.current).toBeNull()
+    expect(objectRefB.current).toBeNull()
+  })
+
+  it('should ignore undefined and null refs', () => {
+    const node = document.createElement('div')
+    const cleanup = vi.fn()
+    const refWithCleanup = vi.fn(() => cleanup)
+
+    expect(() => composeRefs(refWithCleanup, undefined, null as any)(node)).not.toThrow()
+  })
+
+  it('should do nothing and return no cleanup when every ref is nullish', () => {
+    const node = document.createElement('div')
+
+    const dispose = composeRefs(undefined, null as any, undefined)(node)
+
+    expect(dispose).toBeUndefined()
+  })
+
+  it('should not call a plain callback ref again on cleanup beyond the initial null call', () => {
+    const node = document.createElement('div')
+    const cleanup = vi.fn()
+    const refWithCleanup = vi.fn(() => cleanup)
+    const plainCallbackRef = vi.fn()
+
+    const dispose = composeRefs(refWithCleanup, plainCallbackRef)(node) as VoidFunction | undefined
+    dispose?.()
+
+    expect(plainCallbackRef).toHaveBeenCalledTimes(2)
+    expect(plainCallbackRef).toHaveBeenNthCalledWith(1, node)
+    expect(plainCallbackRef).toHaveBeenNthCalledWith(2, null)
+  })
 })
 
 describe('Util: useComposedRefs', () => {
@@ -73,4 +167,52 @@ describe('Util: useComposedRefs', () => {
     expect(firstRef).toHaveBeenCalledWith(null)
     expect(secondRef).toHaveBeenCalledWith(node)
   })
+
+  it('should tear down the old cleanup-ref bundle and set up the new one when the ref set changes', () => {
+    const cleanup = vi.fn()
+    const refWithCleanup = vi.fn(() => cleanup)
+    const plainCallbackRef = vi.fn()
+
+    const ComponentUnderTest = (props: { includeCleanupRef: boolean }) => {
+      const composedRefs = useComposedRefs(props.includeCleanupRef ? refWithCleanup : undefined, plainCallbackRef)
+      return <div data-testid="node" ref={composedRefs} />
+    }
+
+    const { rerender } = render(<ComponentUnderTest includeCleanupRef />)
+    const node = screen.getByTestId('node')
+
+    expect(refWithCleanup).toHaveBeenCalledWith(node)
+    expect(plainCallbackRef).toHaveBeenCalledWith(node)
+    expect(plainCallbackRef).not.toHaveBeenCalledWith(null)
+
+    rerender(<ComponentUnderTest includeCleanupRef={false} />)
+
+    expect(cleanup).toHaveBeenCalledTimes(1)
+    expect(plainCallbackRef).toHaveBeenCalledWith(null)
+    expect(plainCallbackRef).toHaveBeenLastCalledWith(node)
+  })
+
+  it('should detach plain callback and object refs on unmount when a sibling ref uses cleanup', () => {
+    const cleanup = vi.fn()
+    const refWithCleanup = vi.fn(() => cleanup)
+    const plainCallbackRef = vi.fn()
+
+    const ComponentUnderTest = (props: { objectRef: RefObject<HTMLDivElement | null> }) => {
+      const composedRefs = useComposedRefs(refWithCleanup, plainCallbackRef, props.objectRef)
+      return <div data-testid="node" ref={composedRefs} />
+    }
+
+    const objectRef = { current: null as HTMLDivElement | null }
+    const { unmount } = render(<ComponentUnderTest objectRef={objectRef} />)
+    const node = screen.getByTestId('node')
+
+    expect(objectRef.current).toBe(node)
+    expect(plainCallbackRef).toHaveBeenCalledWith(node)
+
+    unmount()
+
+    expect(cleanup).toHaveBeenCalledTimes(1)
+    expect(plainCallbackRef).toHaveBeenCalledWith(null)
+    expect(objectRef.current).toBeNull()
+  })
 })

packages/react/src/utils/compose-refs.ts

@@ -6,19 +6,26 @@ type PossibleRef<T> = Ref<T | null> | undefined
 export function composeRefs<T>(...refs: PossibleRef<T>[]): RefCallback<T> {
   return (node) => {
     const cleanUps: VoidFunction[] = []
+    let hasCustomCleanUp = false
 
     for (const ref of refs) {
       if (typeof ref === 'function') {
         const cb = ref(node)
         if (typeof cb === 'function') {
+          hasCustomCleanUp = true
           cleanUps.push(cb)
+        } else {
+          cleanUps.push(() => ref(null))
         }
       } else if (ref) {
         ref.current = node
+        cleanUps.push(() => {
+          ref.current = null
+        })
       }
     }
 
-    if (cleanUps.length) {
+    if (hasCustomCleanUp) {
       return () => {
         for (const cleanUp of cleanUps) {
           cleanUp()

packages/solid/src/components/toggle-group/examples/with-tooltip.tsx

@@ -0,0 +1,47 @@
+import { ToggleGroup } from '@ark-ui/solid/toggle-group'
+import { Tooltip, useTooltip } from '@ark-ui/solid/tooltip'
+import { BoldIcon, ItalicIcon, UnderlineIcon } from 'lucide-solid'
+import { For } from 'solid-js'
+import { Portal } from 'solid-js/web'
+import styles from 'styles/toggle-group.module.css'
+import tooltipStyles from 'styles/tooltip.module.css'
+
+const items = [
+  { value: 'bold', label: 'Bold', icon: BoldIcon },
+  { value: 'italic', label: 'Italic', icon: ItalicIcon },
+  { value: 'underline', label: 'Underline', icon: UnderlineIcon },
+]
+
+const getTriggerId = (value?: string) => `toggle-item:${value}`
+
+export const WithTooltip = () => {
+  const tooltip = useTooltip({ ids: { trigger: getTriggerId } })
+
+  return (
+    <Tooltip.RootProvider value={tooltip}>
+      <ToggleGroup.Root defaultValue={['bold']} ids={{ item: getTriggerId }} class={styles.Root}>
+        <For each={items}>
+          {(item) => (
+            <ToggleGroup.Item
+              value={item.value}
+              aria-label={item.label}
+              class={styles.Item}
+              asChild={(itemProps) => (
+                <Tooltip.Trigger value={item.value} {...itemProps()}>
+                  <item.icon />
+                </Tooltip.Trigger>
+              )}
+            />
+          )}
+        </For>
+      </ToggleGroup.Root>
+      <Portal>
+        <Tooltip.Positioner>
+          <Tooltip.Content class={tooltipStyles.Content}>
+            {items.find((item) => item.value === tooltip().triggerValue)?.label}
+          </Tooltip.Content>
+        </Tooltip.Positioner>
+      </Portal>
+    </Tooltip.RootProvider>
+  )
+}

packages/solid/src/components/toggle-group/toggle-group.stories.tsx

@@ -10,3 +10,4 @@ export { Basic } from './examples/basic.tsx'
 export { Controlled } from './examples/controlled.tsx'
 export { Multiple } from './examples/multiple.tsx'
 export { RootProvider } from './examples/root-provider.tsx'
+export { WithTooltip } from './examples/with-tooltip.tsx'

packages/svelte/src/lib/components/toggle-group/examples/with-tooltip.svelte

@@ -0,0 +1,42 @@
+<script lang="ts">
+  import { Portal } from '@ark-ui/svelte/portal'
+  import { ToggleGroup } from '@ark-ui/svelte/toggle-group'
+  import { Tooltip, useTooltip } from '@ark-ui/svelte/tooltip'
+  import BoldIcon from 'lucide-svelte/icons/bold'
+  import ItalicIcon from 'lucide-svelte/icons/italic'
+  import UnderlineIcon from 'lucide-svelte/icons/underline'
+  import styles from 'styles/toggle-group.module.css'
+  import tooltipStyles from 'styles/tooltip.module.css'
+
+  const items = [
+    { value: 'bold', label: 'Bold', icon: BoldIcon },
+    { value: 'italic', label: 'Italic', icon: ItalicIcon },
+    { value: 'underline', label: 'Underline', icon: UnderlineIcon },
+  ]
+
+  const getTriggerId = (value?: string) => `toggle-item:${value}`
+
+  const id = $props.id()
+  const tooltip = useTooltip({ id, ids: { trigger: getTriggerId } })
+</script>
+
+<Tooltip.RootProvider value={tooltip}>
+  <ToggleGroup.Root defaultValue={['bold']} ids={{ item: getTriggerId }} class={styles.Root}>
+    {#each items as item (item.value)}
+      <ToggleGroup.Item value={item.value} aria-label={item.label} class={styles.Item}>
+        {#snippet asChild(itemProps)}
+          <Tooltip.Trigger value={item.value} {...itemProps()}>
+            <item.icon />
+          </Tooltip.Trigger>
+        {/snippet}
+      </ToggleGroup.Item>
+    {/each}
+  </ToggleGroup.Root>
+  <Portal>
+    <Tooltip.Positioner>
+      <Tooltip.Content class={tooltipStyles.Content}>
+        {items.find((item) => item.value === tooltip().triggerValue)?.label}
+      </Tooltip.Content>
+    </Tooltip.Positioner>
+  </Portal>
+</Tooltip.RootProvider>

packages/svelte/src/lib/components/toggle-group/toggle-group.stories.ts

@@ -3,6 +3,7 @@ import BasicExample from './examples/basic.svelte'
 import ControlledExample from './examples/controlled.svelte'
 import MultipleExample from './examples/multiple.svelte'
 import RootProviderExample from './examples/root-provider.svelte'
+import WithTooltipExample from './examples/with-tooltip.svelte'
 
 const meta: Meta = {
   title: 'Components / Toggle Group',
@@ -33,3 +34,9 @@ export const RootProvider = {
     Component: RootProviderExample,
   }),
 }
+
+export const WithTooltip = {
+  render: () => ({
+    Component: WithTooltipExample,
+  }),
+}

packages/vue/src/components/toggle-group/examples/with-tooltip.vue

@@ -0,0 +1,43 @@
+<script setup lang="ts">
+import { ToggleGroup } from '@ark-ui/vue/toggle-group'
+import { Tooltip, useTooltip } from '@ark-ui/vue/tooltip'
+import { BoldIcon, ItalicIcon, UnderlineIcon } from 'lucide-vue-next'
+import { computed } from 'vue'
+import styles from 'styles/toggle-group.module.css'
+import tooltipStyles from 'styles/tooltip.module.css'
+
+const items = [
+  { value: 'bold', label: 'Bold', icon: BoldIcon },
+  { value: 'italic', label: 'Italic', icon: ItalicIcon },
+  { value: 'underline', label: 'Underline', icon: UnderlineIcon },
+]
+
+const getTriggerId = (value?: string) => `toggle-item:${value}`
+
+const tooltip = useTooltip({ ids: { trigger: getTriggerId } })
+const activeLabel = computed(() => items.find((item) => item.value === tooltip.value.triggerValue)?.label)
+</script>
+
+<template>
+  <Tooltip.RootProvider :value="tooltip">
+    <ToggleGroup.Root :defaultValue="['bold']" :ids="{ item: getTriggerId }" :class="styles.Root">
+      <ToggleGroup.Item
+        v-for="item in items"
+        :key="item.value"
+        :value="item.value"
+        :aria-label="item.label"
+        :class="styles.Item"
+        asChild
+      >
+        <Tooltip.Trigger :value="item.value">
+          <component :is="item.icon" />
+        </Tooltip.Trigger>
+      </ToggleGroup.Item>
+    </ToggleGroup.Root>
+    <Teleport to="body">
+      <Tooltip.Positioner>
+        <Tooltip.Content :class="tooltipStyles.Content">{{ activeLabel }}</Tooltip.Content>
+      </Tooltip.Positioner>
+    </Teleport>
+  </Tooltip.RootProvider>
+</template>