[Bug]: Invalid TypeScript definition for multiple components (DataTable, Modal)

[rwibm]

Package

@carbon/react

Browser

No response

Package version

1.38.0

React version

18.2.0

Description

The typescript definition for the getSelectionProps DataTable does not match the JS implementation, which completely breaks strict builds when using the "select all" feature without manually disabling type-checking using comments or casts, which we shouldn't be encouraging.

In TypeScript the getSelectionProps is defined as:

getSelectionProps: (getSelectionPropsArgs: {
        onClick?: (e: MouseEvent) => void;
        row: DataTableRow<ColTypes>;
        [key: string]: unknown;
    }) => {
        ariaLabel: string;
        checked: boolean | undefined;
        disabled?: boolean | undefined;
        id: string;
        indeterminate?: boolean;
        name: string;
        onSelect: (e: MouseEvent) => void;
        radio?: boolean | null;
        [key: string]: unknown;
    };

Which causes type checking to error out when getSelectionPropsArgs is undefined, as is expected when using this in TableSelectAll as the examples do.

Reproduction/example

N/A

Steps to reproduce

Try to use TS in strict mode for the DataTable example code found in the storybook for selectable rows and it won't compile without errors.

But to fix it is super easy! Just need to add the missing optional question mark after getSelectionPropsArgs:

getSelectionProps: (getSelectionPropsArgs?: {

Suggested Severity

Severity 3 = User can complete task, and/or has a workaround within the user experience of a given component.

Application/PAL

IBM Internal

Code of Conduct

• [X] I agree to follow this project's Code of Conduct
• [X] I checked the current issues for duplicate problems

2024 Update:

This is a deeper issue with the TypeScript support currently implemented, adding more details here to try and clean up these issues as I accidentally reported multiple related issues.

See update here: https://github.com/carbon-design-system/carbon/issues/13091#issuecomment-1989729035

According to TypeScript the props returned by the getHeaderProps function on data tables has a different onClick event handler type to the one expected by the header component.

Using the example from the storybook gives the following typescript error:

error TS2322: Type '{ children: ReactNode; isSortable: boolean | undefined; isSortHeader: boolean; key: string; onClick: (e: MouseEvent) => void; sortDirection: DataTableSortState; }' is not assignable to type 'TableHeaderProps'.
  Types of property 'onClick' are incompatible.
    Type '(e: MouseEvent) => void' is not assignable to type 'MouseEventHandler<HTMLButtonElement>'.

LINE: <TableHeader {...getHeaderProps({ header, isSortable: true })}>

EDIT:

This also appears to affect other parts of the DataTable components, I am now getting errors for the ToolBarSearch component onChange event handler having the wrong type when passing in the onInputChange prop from the render function directly.

 error TS2322: Type '(e: ChangeEvent<HTMLInputElement>, defaultValue?: string | undefined) => void' is not assignable to type '(event: "" | ChangeEvent<HTMLInputElement>, value?: string | undefined) => void'.
  Types of parameters 'e' and 'event' are incompatible.
    Type '"" | ChangeEvent<HTMLInputElement>' is not assignable to type 'ChangeEvent<HTMLInputElement>'.
      Type 'string' is not assignable to type 'ChangeEvent<HTMLInputElement>'.

LINE: <TableToolbarSearch onChange={onInputChange} />

Weirdly, the search bar error does not appear on stackblitz, it knows the signatures are different, but doesn't show any errors. But it also thinks that "'DataTable' cannot be used as a JSX component.", so I'm not sure if my example is working correctly on there, but "tsc" locally gives the above error.

EDIT2:

This also affects the TableExpandRow component too, the props don't match.

 error TS2322: Type '{ children: (Element | Element[])[]; ariaLabel: string; "aria-label": string; disabled: boolean | undefined; isExpanded?: boolean | undefined; isSelected?: boolean | undefined; key: string; onExpand: (e: MouseEvent) => void; }' is not assignable to type 'TableExpandRowProps'.
  Types of property 'isExpanded' are incompatible.
    Type 'boolean | undefined' is not assignable to type 'boolean'.
      Type 'undefined' is not assignable to type 'boolean'.

LINE: <TableExpandRow {...getRowProps({ row })}>

Reproduction/example

https://stackblitz.com/edit/react-ts-dxtfrs?file=App.tsx

[parthrc]

Hey can I work on this?

[rwibm]

Might be worth mentioning that ModalFooter also has the same problem with the "children" prop, the docs state it's supposed to be optional and has examples showing it used that way, but the typescript definition marks children as a required prop for ModalFooter :(

[alexgubanow]

Hi @rwibm please rename bug into something like "DataTable has bad types". @carbon/react 1.39 react 18.2.0 I have faced more wrong types in this component: getHeaderProps has bad type of onClick getSelectionProps has bad type of onClick and checked.

In same time, im not sure if making getSelectionPropsArgs nullable makes sense, but this is how my select all looks like so far:

                <TableSelectAll
                  id='lalala'
                  name='lalala'
                  checked={selectedRows.length == rows.length}
                  onSelect={() => { rows.forEach((row) => selectRow(row.id)); }} />

[rwibm]

I think this is a larger issue with Carbon's TypeScript support in general (it's relatively new for carbon), there seems to be a few definitions that are invalid for several components.

I think the carbon team just needs to give somebody a few days to go through all of the definitions and make sure they match what the docs/implementation says the component can do. But it seems they're relying heavily on community contributions at the moment :(

[tay1orjones]

Hey there, I know this is frustrating and I wish we had the capacity to tackle issues like this promptly. Unfortunately our team is focused on things right now and, as you mention, we're relying on our excellent network of contributors to add types to the react components.

We'd welcome a PR for this one if anyone is able.

[KunjMaheshwari]

Please Assign this issue to me. I'm new to Open Source.

[nkn2022]

With the latest carbon version, I notice that the ModalFooter component is not allowed to be defined without the children prop in typescript which does not match the doc.

Looks like an issue in the TypeScript type definition.

Is this the right issue to track this problem?

[Laloses]

I saw the warning for end of support for v10 ends in September, so I decided to update now that I have free time. And what I found is that v11 is not ready to be used with typescript.

I hope 100% coverage for TS is completed before September 😢

[imp-dance]

[...] And what I found is that v11 is not ready to be used with typescript.

Migrated to newest version of Carbon a few weeks ago and just want to chime in and say I disagree with this strongly. I've had to declare types for ~4 components I think? Not a big deal at all. Typescript support at this point is not much worse than the definitely typed types from the previous versions.

[kubijo]

I believe that another thing that is relevant to this issue is ariaLabel in several places like OverflowMenu where ariaLabel is declared deprecated, but required (missing ?).

It either nags you that you missed a required prop or that you used a deprecated one...