Material-ui: [Tabs] Issues with onChange TypeScript types

[ianschmitz]

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Wrapping the Tabs component results in TypeScript emitting the wrong types for the onChange prop:

Type '(event: ChangeEvent<{}>, value: number) => void' is not assignable to type '((event: ChangeEvent<{}>, value: any) => void) & ((event: FormEvent<HTMLButtonElement>) => void)'.
  Type '(event: ChangeEvent<{}>, value: number) => void' is not assignable to type '(event: FormEvent<HTMLButtonElement>) => void'.ts(2322)

The same problem looks to be appearing for action. It appears to be intersecting the types from ButtonBases props. Maybe a union type was intended?

(((instance: TabsActions | null) => void) & ((instance: ButtonBaseActions | null) => void)) | (((instance: TabsActions | null) => void) & React.RefObject<ButtonBaseActions>) | (React.RefObject<...> & ((instance: ButtonBaseActions | null) => void)) | (React.RefObject<...> & React.RefObject<...>) | null | undefined

Expected Behavior 🤔

The type of TabsProps.onChange should be ((event: React.ChangeEvent<{}>, value: any) => void) | undefined. I believe this regression may have been caused by #16487.

Steps to Reproduce 🕹

See https://codesandbox.io/s/upbeat-grass-gc0i9 for the TypeScript error and example wrapping of Tabs component.

Context 🔦

It's quite common for us to wrap up the MUI components with our own customizations and export them as common components to use throughout our applications. In this case after upgrading MUI's minor version recently this started causing TypeScript errors everywhere we've used Tabs.

Your Environment 🌎

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.4.2 |
| React | v16.9.0 |
| Browser | |
| TypeScript | v3.6.3 |
| etc. | |

[ianschmitz]

I think there's actually a lot more going on across many components. I'm encountering the same issue in a number of places. For example using Link:

function onClick(event: React.MouseEvent<HTMLButtonElement>) {}

return (<Link component="button" onClick={onClick}>
  Hey!
</Link>);

has error:

Type '(event: MouseEvent<HTMLButtonElement, MouseEvent>) => void' is not assignable to type '((event: MouseEvent<HTMLAnchorElement, MouseEvent>) => void) & ((event: MouseEvent<HTMLElement, MouseEvent>) => void)'.
  Type '(event: MouseEvent<HTMLButtonElement, MouseEvent>) => void' is not assignable to type '(event: MouseEvent<HTMLAnchorElement, MouseEvent>) => void'.
    Types of parameters 'event' and 'event' are incompatible.

Sample: https://codesandbox.io/s/create-react-app-with-typescript-ztfq4

/cc @ypresto

[ianschmitz]

Updated title to reflect that the issue isn't just within Tabs.

[oliviertassinari]

@ianschmitz I can't reproduce the issue on your codesandbox. Can you?

[ianschmitz]

I'm still seeing in my sandbox in the OP on latest MUI (4.7.1).

[b-zurg]

I'm also getting the following error:

TS2322: Type '(event: React.ChangeEvent<{}>, newValue: string) => void' is not assignable to type '((event: ChangeEvent<{}>, value: any) => void) & ((event: FormEvent<HTMLButtonElement>) => void)'.   Type '(event: React.ChangeEvent<{}>, newValue: string) => void' is not assignable to type '(event: FormEvent<HTMLButtonElement>) => void'

I think the key part of this error is the expected type: ((event: ChangeEvent<{}>, value: any) => void) & ((event: FormEvent<HTMLButtonElement>) => void)

The & should be an | it seems as there's no valid function that would match both of those function signatures at the same time that I can see.

[SpotHimesh]

Having the same error, does anyone have a better fix/hack for it than this?

handleChange = compose((event: React.ChangeEvent<{}>) => {
      // do nothing.
    }, (event: React.FormEvent<HTMLButtonElement>, newValue: any) => {
      setValue(newValue);
});

[JesalR]

We are also having this issue
Our (temporary) solve is to use

import Tabs, { TabsProps } from ""@material-ui/core/Tabs"
const HnTabs: typeof Tabs = (props: TabsProps) => <Tabs {...props} />;

But we'd like a better solution, this doesn't feel good

[oliviertassinari]

@ianschmitz I have seen the exact same bug in #20191. What do you think of this patch https://github.com/mui-org/material-ui/issues/20191#issuecomment-601467065?

[cristian-eriomenco]

@oliviertassinari I thought i was going crazy here is a sandbox showing 2 approaches both breaking something and fixing something else.

[ianschmitz]

@ianschmitz I have seen the exact same bug in #20191. What do you think of this patch #20191 (comment)?

@oliviertassinari a quick glance _seems_ like that would improve things. To be honest some of those types make me go cross eyed when i read them so it's hard to say without writing out a bunch of test cases 😛

[b-zurg]

I'm just doing this right now. It's messy but I don't really know a better solution

interface TabsPropsFixed extends Omit<TabsProps, "onChange"> {
  onChange:
    | ((event: ChangeEvent<{}>, value: any) => void)
    | ((event: FormEvent<HTMLButtonElement>) => void);
}
export const Tabs: React.FC<TabsPropsFixed> = props => <Tabs {...props}/>

[eps1lon]

Good news: The suggested type was definitely wrong on our part.
Bad news: React.ChangeEvent is not the (technically) correct type (Though React.ChangeEvent<{}> will work incidentally. It's only valid structurally but has implications that don't hold true such as event.target === event.currentTarget.)

We're not creating a custom, react change event in the Tabs component. We're simply passing along whatever event triggered the change in value. I won't go into detail what specific type of events this could be so that we're not needlessly restricting our implementation.

It's a bit unfortunate since it's highly unlikely that you ever relied on specifics of ChangeEvent simply because they don't matter in our case (has something to do with target vs. currentTarget).

I suggest using function handleTabsChange(event: React.SyntheticEvent, tabsValue: unknown) {} since that is going to be the type in v5.

[walltex]

@eps1lon I still do have the issue. I tried both solutions, nothing works for me.

[email protected]

UPD: I just realised that this fix was merged into next branch that is 5.* alpha. Is it possible to have this fix cherry-picked to 4.* by any chance?

[eps1lon]

Is it possible to have this fix cherry-picked to 4.* by any chance?

We're only backporting important or security fixes at this stage.

[vinikatyal]

@eps1lon I still do have the issue. I tried both solutions, nothing works for me.

[email protected]

UPD: I just realised that this fix was merged into next branch that is 5.* alpha. Is it possible to have this fix cherry-picked to 4.* by any chance?

Were you able to figure this out?

[vinikatyal]

function handleTabsChange(event: React.SyntheticEvent, tabsValue: unknown) {}

This worked for me

[iiskaandar]

Still have this issue, and when I am reading "event.target.value" or "event.currentTarget.value" there is "undefined" or "null".

[vinikatyal]

Still have this issue, and when I am reading "event.target.value" or "event.currentTarget.value" there is "undefined" or "null".

export interface ITabsProps extends Omit {
children?: React.ReactNode;
textColor?: TabsProps['textColor'];
onChange: (event: React.ChangeEvent<{}>, newValue: number) => void;
}

@iiskaandar I used this