Move the ref from accessor arg to a returned function

.changeset/rude-melons-own.md

@@ -0,0 +1,6 @@
+---
+"@solid-aria/button": patch
+"@solid-aria/focus": patch
+---
+
+Fix user props not being passed down to the returned props object. (#69)

packages/button/src/createButton.ts

@@ -14,76 +14,41 @@
  * OF ANY KIND, either express or implied. See the License for the specific language
  * governing permissions and limitations under the License.
  */
+/* eslint-disable solid/reactivity */
 
-import { createFocusable } from "@solid-aria/focus";
+import { createFocusable, CreateFocusableProps } from "@solid-aria/focus";
 import { createPress } from "@solid-aria/interactions";
 import { ElementType } from "@solid-aria/types";
-import { filterDOMProps } from "@solid-aria/utils";
 import { combineProps } from "@solid-primitives/props";
-import { Accessor, createMemo, JSX, mergeProps, splitProps } from "solid-js";
+import { Accessor, JSX, mergeProps, splitProps } from "solid-js";
 
-import { AriaButtonProps } from "./types";
+import { AriaButtonProps, ElementOfType, PropsOfType } from "./types";
+
+export interface ButtonAria<T extends ElementType> {
+  /**
+   * A ref to apply onto the target element.
+   * Merge the given `props.ref` and all parents `PressResponder` refs.
+   */
+  ref: (el: ElementOfType<T>) => void;
 
-export interface ButtonAria<T> {
   /**
    * Props for the button element.
    */
-  buttonProps: T;
+  buttonProps: Omit<PropsOfType<T>, "ref">;
 
   /**
    * Whether the button is currently pressed.
    */
   isPressed: Accessor<boolean>;
 }
 
-// Order with overrides is important: 'button' should be default
-export function createButton(
-  props: AriaButtonProps<"button">,
-  ref: Accessor<HTMLButtonElement | undefined>
-): ButtonAria<JSX.ButtonHTMLAttributes<HTMLButtonElement>>;
-
-export function createButton(
-  props: AriaButtonProps<"a">,
-  ref: Accessor<HTMLAnchorElement | undefined>
-): ButtonAria<JSX.AnchorHTMLAttributes<HTMLAnchorElement>>;
-
-export function createButton(
-  props: AriaButtonProps<"div">,
-  ref: Accessor<HTMLDivElement | undefined>
-): ButtonAria<JSX.HTMLAttributes<HTMLDivElement>>;
-
-export function createButton(
-  props: AriaButtonProps<"input">,
-  ref: Accessor<HTMLInputElement | undefined>
-): ButtonAria<JSX.InputHTMLAttributes<HTMLInputElement>>;
-
-export function createButton(
-  props: AriaButtonProps<"span">,
-  ref: Accessor<HTMLSpanElement | undefined>
-): ButtonAria<JSX.HTMLAttributes<HTMLSpanElement>>;
-
-export function createButton(
-  props: AriaButtonProps<ElementType>,
-  ref: Accessor<HTMLElement | undefined>
-): ButtonAria<JSX.HTMLAttributes<HTMLElement>>;
-
 /**
  * Provides the behavior and accessibility implementation for a button component. Handles mouse, keyboard, and touch interactions,
  * focus behavior, and ARIA props for both native button elements and custom element types.
  * @param props - Props to be applied to the button.
- * @param ref - A ref to a DOM element for the button.
  */
-export function createButton(
-  props: AriaButtonProps<ElementType>,
-  ref: Accessor<any>
-): ButtonAria<JSX.HTMLAttributes<any>> {
-  const defaultProps: AriaButtonProps<"button"> = {
-    elementType: "button",
-    type: "button"
-  };
-
-  // eslint-disable-next-line solid/reactivity
-  props = mergeProps(defaultProps, props);
+export function createButton<T extends ElementType>(props: AriaButtonProps<T>): ButtonAria<T> {
+  props = mergeProps({ elementType: "button", type: "button" }, props) as typeof props;
 
   const additionalButtonElementProps: JSX.ButtonHTMLAttributes<any> = {
     get type() {
@@ -120,12 +85,8 @@ export function createButton(
       }
     };
 
-  const additionalProps = mergeProps(
-    createMemo(() => {
-      return props.elementType === "button"
-        ? additionalButtonElementProps
-        : additionalOtherElementProps;
-    })
+  const additionalProps = mergeProps(() =>
+    props.elementType === "button" ? additionalButtonElementProps : additionalOtherElementProps
   );
 
   const [createPressProps] = splitProps(props, [
@@ -137,11 +98,12 @@ export function createButton(
     "preventFocusOnPress"
   ]);
 
-  const { pressProps, isPressed } = createPress(createPressProps);
+  const { pressProps, isPressed, ref: pressRef } = createPress(createPressProps);
 
-  const { focusableProps } = createFocusable(props, ref);
-
-  const domProps = filterDOMProps(props, { labelable: true });
+  // createFocusable has problems with the `props` type because if you pass a component as the `elementType` prop, there is no telling if the `ref` will be a HTML Element.
+  const { focusableProps, ref: focusRef } = createFocusable(
+    props as CreateFocusableProps<HTMLElement>
+  );
 
   const baseButtonProps: JSX.HTMLAttributes<any> = {
     get "aria-haspopup"() {
@@ -173,9 +135,15 @@ export function createButton(
     additionalProps,
     focusableProps,
     pressProps,
-    domProps,
     baseButtonProps
-  );
-
-  return { buttonProps, isPressed };
+  ) as Omit<PropsOfType<T>, "ref">;
+
+  return {
+    buttonProps,
+    isPressed,
+    ref: el => {
+      focusRef(el as HTMLElement);
+      pressRef(el as HTMLElement);
+    }
+  };
 }

packages/button/src/types.ts

@@ -23,7 +23,30 @@ import {
   FocusableProps,
   PressEvents
 } from "@solid-aria/types";
-import { JSX } from "solid-js";
+import { Component, JSX, Ref } from "solid-js";
+
+// TODO: these probably should be added to @solid-aria/types
+
+/**
+ * Infers the type of `ref` element from the `elementType` prop.
+ * @example
+ * "button" -> HTMLButtonElement
+ * "div" -> HTMLDivElement
+ * Component<{ ref: Ref<HTMLInputElement> }> -> HTMLInputElement
+ */
+export type ElementOfType<T extends ElementType> = T extends keyof JSX.IntrinsicElements
+  ? JSX.IntrinsicElements[T] extends JSX.CustomAttributes<infer U>
+    ? U
+    : never
+  : T extends Component<{ ref: Ref<infer U> }>
+  ? U
+  : never;
+
+export type PropsOfType<T extends ElementType> = T extends keyof JSX.IntrinsicElements
+  ? JSX.IntrinsicElements[T]
+  : T extends Component<any>
+  ? Parameters<T>[0]
+  : never;
 
 interface ButtonProps extends PressEvents, FocusableProps {
   /**
@@ -38,6 +61,11 @@ interface ButtonProps extends PressEvents, FocusableProps {
 }
 
 export interface AriaButtonElementTypeProps<T extends ElementType = "button"> {
+  /**
+   * A ref to the target element.
+   */
+  ref?: Ref<ElementOfType<T>>;
+
   /**
    * The HTML element or SolidJS component used to render the button, e.g. 'div', 'a', or `Link`.
    * @default 'button'

packages/button/test/createButton.test.tsx

@@ -15,22 +15,22 @@
  * governing permissions and limitations under the License.
  */
 
+import { JSX } from "solid-js";
+
 import { createButton } from "../src";
 import { AriaButtonProps } from "../src/types";
 
 describe("createButton", () => {
   it("handles defaults", () => {
-    let ref: any;
     const props = {};
-    const { buttonProps } = createButton(props, () => ref);
+    const { buttonProps } = createButton(props);
 
     expect(typeof buttonProps.onClick).toBe("function");
   });
 
   it("handles elements other than button", () => {
-    let ref: any;
     const props: AriaButtonProps<"a"> = { elementType: "a" };
-    const { buttonProps } = createButton(props, () => ref);
+    const { buttonProps } = createButton(props);
 
     expect(buttonProps.role).toBe("button");
     expect(buttonProps.tabIndex).toBe(0);
@@ -41,9 +41,8 @@ describe("createButton", () => {
   });
 
   it("handles elements other than button disabled", () => {
-    let ref: any;
     const props: AriaButtonProps<"a"> = { elementType: "a", isDisabled: true };
-    const { buttonProps } = createButton(props, () => ref);
+    const { buttonProps } = createButton(props);
 
     expect(buttonProps.role).toBe("button");
     expect(buttonProps.tabIndex).toBeUndefined();
@@ -54,25 +53,22 @@ describe("createButton", () => {
   });
 
   it("handles the rel attribute on anchors", () => {
-    let ref: any;
     const props: AriaButtonProps<"a"> = { elementType: "a", rel: "noopener noreferrer" };
-    const { buttonProps } = createButton(props, () => ref);
+    const { buttonProps } = createButton(props);
 
     expect(buttonProps.rel).toBe("noopener noreferrer");
   });
 
   it("handles the rel attribute as a string on anchors", () => {
-    let ref: any;
     const props: AriaButtonProps<"a"> = { elementType: "a", rel: "search" };
-    const { buttonProps } = createButton(props, () => ref);
+    const { buttonProps } = createButton(props);
 
     expect(buttonProps.rel).toBe("search");
   });
 
   it("handles input elements", () => {
-    let ref: any;
     const props: AriaButtonProps<"input"> = { elementType: "input", isDisabled: true };
-    const { buttonProps } = createButton(props, () => ref);
+    const { buttonProps } = createButton(props);
 
     expect(buttonProps.role).toBe("button");
     expect(buttonProps.tabIndex).toBeUndefined();
@@ -88,4 +84,27 @@ describe("createButton", () => {
     // @ts-ignore
     expect(buttonProps.rel).toBeUndefined();
   });
+
+  it("user props are passed down to the returned button props", () => {
+    const onMouseMove = jest.fn();
+    const props: AriaButtonProps<"button"> & JSX.IntrinsicElements["button"] = {
+      children: "Hello",
+      class: "test-class",
+      onMouseMove,
+      style: { color: "red" }
+    };
+
+    const { buttonProps } = createButton(props);
+
+    expect(buttonProps.children).toBe("Hello");
+    expect(buttonProps.class).toBe("test-class");
+    expect(buttonProps.style).toEqual({ color: "red" });
+
+    expect(buttonProps).toHaveProperty("onMouseMove");
+    expect(typeof buttonProps.onMouseMove).toBe("function");
+
+    // @ts-expect-error
+    buttonProps.onMouseMove();
+    expect(onMouseMove).toHaveBeenCalled();
+  });
 });

packages/focus/src/createFocusable.ts

@@ -23,9 +23,16 @@ import {
 } from "@solid-aria/interactions";
 import { combineProps } from "@solid-primitives/props";
 import { access, MaybeAccessor } from "@solid-primitives/utils";
-import { Accessor, createSignal, JSX, onMount } from "solid-js";
+import { createSignal, JSX, onMount, Ref, splitProps } from "solid-js";
+
+export interface CreateFocusableProps<El extends HTMLElement>
+  extends CreateFocusProps,
+    CreateKeyboardProps {
+  /**
+   * A ref to the target element.
+   */
+  ref?: Ref<El>;
 
-export interface CreateFocusableProps extends CreateFocusProps, CreateKeyboardProps {
   /**
    * Whether focus should be disabled.
    */
@@ -45,7 +52,13 @@ export interface CreateFocusableProps extends CreateFocusProps, CreateKeyboardPr
   excludeFromTabOrder?: MaybeAccessor<boolean | undefined>;
 }
 
-export interface FocusableResult {
+export interface FocusableResult<El extends HTMLElement> {
+  /**
+   * A ref to apply onto the target element.
+   * Merge the given `props.ref` and all parents `PressResponder` refs.
+   */
+  ref: (el: El) => void;
+
   /**
    * Props to spread onto the target element.
    */
@@ -57,26 +70,30 @@ export interface FocusableResult {
 /**
  * Make an element focusable, capable of auto focus and excludable from tab order.
  */
-export function createFocusable(
-  props: CreateFocusableProps,
-  ref: Accessor<HTMLElement | undefined>
-): FocusableResult {
+export function createFocusable<El extends HTMLElement>(
+  props: CreateFocusableProps<El>
+): FocusableResult<El> {
+  // eslint-disable-next-line solid/reactivity
   const [autofocus, setAutofocus] = createSignal(!!access(props.autofocus));
 
-  const { focusProps } = createFocus(props);
-  const { keyboardProps } = createKeyboard(props);
+  const propsWithoutRef = splitProps(props, ["ref"])[1];
+
+  const { focusProps } = createFocus(propsWithoutRef);
+  const { keyboardProps } = createKeyboard(propsWithoutRef);
 
-  const focusableProps = {
-    ...combineProps(focusProps, keyboardProps),
+  const focusableProps = combineProps(propsWithoutRef, focusProps, keyboardProps, {
     get tabIndex() {
       return access(props.excludeFromTabOrder) && !access(props.isDisabled) ? -1 : undefined;
     }
-  };
+  });
+
+  let target: El | undefined;
+  const ref = (el: El) => (props.ref as (el: El) => void)?.((target = el));
 
   onMount(() => {
-    autofocus() && ref()?.focus();
+    autofocus() && target?.focus();
     setAutofocus(false);
   });
 
-  return { focusableProps };
+  return { focusableProps, ref };
 }