markusenglund / react-switch

A draggable toggle-switch component for React. Check out the demo at:
https://react-switch.netlify.com/
MIT License
1.33k stars 100 forks source link

[Fix] typescript props #46

Closed NoTrebel closed 5 years ago

NoTrebel commented 5 years ago

I finally had time to switch to v5 of this library, however, my type checker threw some errors. After a short investigation I noticed I made a small mistake on the Typescript code I suggested earlier.

This fix correctly exposes the react-switch props (as an intersection type, not a union type). This enables Typescript users to use one of the React.ComponentProps<> conditional types to infer all props of react-switch.

markusenglund commented 5 years ago

It makes sense that the union type would be wrong for this, but I can't reproduce the error.

<Switch
  checked={this.state.checked}
  onChange={this.handleChange}
  tabIndex={1}
  aria-live="polite"
  handleDiameter={28}
  offColor="#08f"
  onColor="#0ff"
  offHandleColor="#0ff"
  onHandleColor="#08f"
  height={40}
  width={70}
  className="react-switch"
  id="small-radius-switch"
/>

This compiles fine and tslint doesn't complain despite using props from both ReactSwitchProps and allowedHTMLinputProps. How can I reproduce?

NoTrebel commented 5 years ago

Interesting. When I tried to reproduce with a very simple use-case, it indeed seems to work just fine. Even just using type test = React.ComponentPropsWithRef<typeof Switch> seems to work fine. However, used together with Omit, I don't get the type information I want. Although reworking Omit might solve the problems as well, the cleaner solution seems to me to fix the Switch component props:

Below example is as simple as I could get it. I'm trying to get all props, without onChange and onBlur:

import * as React from 'react';
import Switch from 'react-switch';

interface IToggleProps {
    onChange: (value: any) => void;
    onBlur: (value: any) => void;
}

type Omit<T, U> = Pick<T, Exclude<keyof T, keyof U>>;

type combinedProps = Omit<React.ComponentPropsWithRef<typeof Switch>, IToggleProps>;

results with the Union type: type combinedProps = { ref: React.Ref<Switch>; key: React.ReactText; }

results with the intersection type:

type combinedProps = {
    ref?: React.Ref<Switch>;
    form?: string;
    style?: React.CSSProperties;
    title?: string;
    pattern?: string;
    checked: boolean;
    disabled?: boolean;
    offColor?: string;
    onColor?: string;
    offHandleColor?: string;
    ... 280 more ...;
    onTransitionEndCapture?: (event: React.TransitionEvent<...>) => void;
}
markusenglund commented 5 years ago

So to be clear, the proposed change gives us completely correct typings?

NoTrebel commented 5 years ago

Very slow response, sorry.

Above types work fine for me and I can't think of any reason why they wouldn't be correct now. I'm not Typescript guru though, so I won't guarantee typings are 100% correct now.

markusenglund commented 5 years ago

Deployed in 5.0.1