material-components / material-components-web-react

Material Components for React (MDC React)
MIT License
2.02k stars 227 forks source link

Interface 'ButtonProps<T>' cannot simultaneously extend types 'AnchorHTMLAttributes<T>' and 'ButtonHTMLAttributes<T>' #781

Open dawsonc623 opened 5 years ago

dawsonc623 commented 5 years ago

Due to a change in @types/react, the definition of ButtonProps does not work anymore. I have not dived into exactly what it would take to fix it, but the only workaround there is right now is for users to stick to @types/react@16.8.8 or lower in their project's dependencies.

ghost commented 5 years ago

The easy fix would be to change it to use HTMLProps like it is done in icon-button. https://github.com/material-components/material-components-web-react/blob/rc0.12.0/packages/icon-button/index.tsx#L45-L46

A more complex approach would be to have a conditional generic like what is done for input in text-field. https://github.com/material-components/material-components-web-react/blob/rc0.12.0/packages/text-field/Input.tsx#L44-L47

moog16 commented 5 years ago

@ben-mckernan yes you're spot on. It should be extending HTMLProps instead of HTMLAttributes. text-field has a good example of this.

I'm not sure what the exact issue is with extending both button and anchor, but conceptually you can't be both at the same time (so this doesn't make sense). If someone would like to open a PR updating the types feel free.

Jason3S commented 5 years ago

The issue is with the type property in both interfaces. They are not of the same type, so extends doesn't know how to merge them. The fix is to pick the one wanted.

Like this:

export interface ButtonProps<T extends ButtonTypes>
  extends Ripple.InjectedProps<T>, React.AnchorHTMLAttributes<T>, React.ButtonHTMLAttributes<T> {
    raised?: boolean;
    unelevated?: boolean;
    outlined?: boolean;
    dense?: boolean;
    disabled?: boolean;
    className?: string;
    icon?: React.ReactElement<React.HTMLProps<HTMLOrSVGElement>>;
    href?: string;
    trailingIcon?: React.ReactElement<React.HTMLProps<HTMLOrSVGElement>>;
    type?: React.ButtonHTMLAttributes<T>['type']; // <-- Add this line to explicitly choose which type is wanted.
}
moog16 commented 5 years ago

Right but this only works for the Button case...if you were using the anchor tag you would need to change this. This issue is to make sure developers can import ButtonProps and it works for both cases. :)