Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useLabel's labelProps fails type-check when used with a div #7269

Open
amitdahan opened this issue Oct 28, 2024 · 1 comment
Open

useLabel's labelProps fails type-check when used with a div #7269

amitdahan opened this issue Oct 28, 2024 · 1 comment

Comments

@amitdahan
Copy link
Contributor

amitdahan commented Oct 28, 2024

Provide a general summary of the issue here

It seems useLabel().labelProps is typed in a way that only works on "label-like" elements.

While type-check works for span, it fails for a div.

🤔 Expected Behavior?

This should pass type-check:

const { labelProps, fieldProps } = useLabel({ labelElementType: 'div' });

return (
  <div {...labelProps}>I'm a label</div>
);

😯 Current Behavior

It fails with the following (quite verbose) type-check error:

Type '{ children: string; id?: string | undefined; role?: AriaRole | undefined; tabIndex?: number | undefined; style?: CSSProperties | undefined; className?: string | undefined; ... 219 more ...; onTransitionStartCapture?: TransitionEventHandler<...> | undefined; } | { ...; }' is not assignable to type 'DetailedHTMLProps<HTMLAttributes<HTMLDivElement>, HTMLDivElement>'.
  Type '{ children: string; form?: string | undefined; htmlFor?: string | undefined; defaultChecked?: boolean | undefined; defaultValue?: string | number | readonly string[] | undefined; suppressContentEditableWarning?: boolean | undefined; ... 271 more ...; onTransitionStartCapture?: TransitionEventHandler<...> | undefined...' is not assignable to type 'DetailedHTMLProps<HTMLAttributes<HTMLDivElement>, HTMLDivElement>'.
    Type '{ children: string; form?: string | undefined; htmlFor?: string | undefined; defaultChecked?: boolean | undefined; defaultValue?: string | number | readonly string[] | undefined; suppressContentEditableWarning?: boolean | undefined; ... 271 more ...; onTransitionStartCapture?: TransitionEventHandler<...> | undefined...' is not assignable to type 'HTMLAttributes<HTMLDivElement>'.
      Types of property 'onToggle' are incompatible.
        Type 'ToggleEventHandler<HTMLLabelElement> | undefined' is not assignable to type 'ToggleEventHandler<HTMLDivElement> | undefined'.
          Type 'ToggleEventHandler<HTMLLabelElement>' is not assignable to type 'ToggleEventHandler<HTMLDivElement>'.
            Property 'align' is missing in type 'HTMLLabelElement' but required in type 'HTMLDivElement'.

💁 Possible Solution

labelProps are currently typed as:

export interface LabelAria {
  labelProps: DOMAttributes | LabelHTMLAttributes<HTMLLabelElement>,
  // ...

While only { id: string, htmlFor?: string } is returned.


One approach can be to narrow down the type, to just what we know we return, but I can see how that can be undesirable, as it can make changing the API surface in the future harder.


Another approach can be to make the returned type depend on the optional labelElementType key:

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAJQKYEMDGMA0cDecAyKARkgDYASAKgLL4CCMMUwRArjEgM5wC+cAZlAgg4AciioMogNwAoUJFi44AEQDy1BkxbsuvAUJGiAAhPQwAtDACeYLgHpOACxQSAJjNlebdgsTJ0zCgACkJgnAA8lHBIAB4cAHZu3ADWSNYQ-HBUtACipEggSAkwlCgA5gByKEXUKGAAfHAAvLJw7XDRcYnJYqT+pKJwAPy4bR0T-SSk+YXFpbZIwwBcnXITfKs44xPtU2SzRSWUi6uUmDu8cvIlSFD86Eh+04HAKFEx8cW9aRlZOfhDvMylUakg6o0xpMBqEIOFVupNIxmGwONwAD7PMgArQo3SRAFA44Vaq1eoAbUoAF0GnIeF5+KwEhhgBAEnBWJwkIRph9ut9UulMtkaICCkdSiSwRCWn0BqIGgAKMBhTirHkBIKw8JRBoASnVA1e70otK8aDZnHgeH2pG13D4zQ5XI1pEVOB4erkFoSVuUtvtq1trvt+idnO5A3dcFtRIWdlWoltQ093st1pjMNVq04YBQCRDqrDzsj02jsfFwNOYlz+ZTXtkPr9NqzcLVcDcwAAboW28WI67ywM4ycE2JO1369cm-B6AAhXL4WURW24AB0G4Dqr4OA3a+DrfCPAaKEz0zPZAi9ltZpnqgAkgA1WWKy4RCcNS4Td-d9cbie9keDTOBArCkG4cAkHAEApFeH5fh0P5drum6Hpwx4gWBEEPMApBwd2n7fvY8ENneADKwR0JUy61uyKFrrRgHoSecC0ReeGOHmCS0kAA

(Notice the change from ElementType to keyof HTMLElementTagNameMap, so we can correctly index it with T)

🖥️ Steps to Reproduce

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wG4AoUSWOAbzgFcBnJAGRQCMkAbOAXzjZcBIuhgBaFFGAoy5cmggA7JvHrdOPAAo4wTAFxwAJsABu7Ltx0Q9-OAF5GLCzwAU6zdwCi3JCCRKMAAqAJ5gSIb4Jqb4-ACUFIoqanAaltZ6hkxgKEouVrpMdo7MbJ7uqZ4+fgHBYREE2bmxfAnySapwTUoOcAA83XQAdCPd+RlMfAB8KF05PWk8fQD03VOJyp3RvX3btCND0eOF07Pbi9wr0etAA

function SomeComponent() {
  const { fieldProps: divFieldProps, labelProps: divLabelProps } = useLabel({
    label: 'This is a div label',
    labelElementType: 'div',
  });
  const { fieldProps: spanFieldProps, labelProps: spanLabelProps } = useLabel({
    label: 'This is a span label',
    labelElementType: 'span',
  });

  return (
    <div>
      <div>
        <span {...spanLabelProps}>This is a span label</span>
        <div {...spanFieldProps}>This is what's being labelled</div>
      </div>
      <div>
        <div {...divLabelProps}>This is a div label</div>
        <div {...divFieldProps}>This is what's being labelled</div>
      </div>
    </div>
  );
}

Version

Latest react-aria.

@yihuiliao
Copy link
Member

I think you're right that useLabel should work with a div. Did some digging and it seems that in the docs, when you look up the props for LabelAriaProps that labelElementType accepts either a 'label' or 'span'. That said, later in the docs it says "However, if you are labeling a non-native form element, be sure to use an element other than a and set the labelElementType prop appropriately" which makes it seem more open-ended.

I'll need to consult with the team to see why this is the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants