Hacker0x01 / react-datepicker

A simple and reusable datepicker component for React
https://reactdatepicker.com/
MIT License
7.89k stars 2.23k forks source link

The `date` parameter in `onChange` from `DatePickerProps` is differently typed in v7.0.0 #4924

Open arendora opened 1 week ago

arendora commented 1 week ago

Describe the bug The date parameter in onChange when taken from DatePickerProps is type Date & [Date | null, Date | null] & Date[].

Before v7.0.0 , the date parameter in onChange from ReactDatePickerProps is typed depending on WithRange and WithMultiple so that types Date | null, Date[] | null or [Date | null, Date | null] were accepted.

onChange(
  date: WithRange extends false | undefined
    ? (WithMultiple extends false | undefined ? Date | null : Date[] | null)
    : [Date | null, Date | null],
  event: React.SyntheticEvent<any> | undefined,
): void;

However, in v7.0.0 the date parameter is now type Date & [Date | null, Date | null] & Date[].

To Reproduce Sandbox link

Expected behavior onChange from DatePickerProps accepts parameter date of type Date.

Screenshots

ts-date-error
talatkuyuk commented 1 week ago

I think there is no bug or problem. It is not Date & [Date | null, Date | null] & Date[] as @arendora described.

The type depends on selectsRange and selectsMultiple.

It is Date | null or [Date | null, Date | null] or Date[] | null depending on selectsRange and selectsMultiple.

(
    | {
        selectsRange?: never;
        selectsMultiple?: never;
        onChange: (
          date: Date | null,
          event?:
            | React.MouseEvent<HTMLElement>
            | React.KeyboardEvent<HTMLElement>,
        ) => void;
      }
    | {
        selectsRange: true;
        selectsMultiple?: never;
        onChange: (
          date: [Date | null, Date | null],
          event?:
            | React.MouseEvent<HTMLElement>
            | React.KeyboardEvent<HTMLElement>,
        ) => void;
      }
    | {
        selectsRange?: never;
        selectsMultiple: true;
        onChange: (
          date: Date[] | null,
          event?:
            | React.MouseEvent<HTMLElement>
            | React.KeyboardEvent<HTMLElement>,
        ) => void;
      }
  );
arendora commented 6 days ago

@talatkuyuk That was my expectation as well. However, you can see in the sandbox link I provided, selectsRange and selectsMultiple are both undefined/falsy so the date parameter for onChange should be date: Date | null however the typing is instead Date & [Date | null, Date | null] & Date[]

talatkuyuk commented 5 days ago

@arendora, don't pass directly onChange of DatePickerProps in Props. Instead, set the type accordingly.

type Props = Omit<DatePickerProps, "onChange"> & {
  label: string;
  onChange: (date: Date | null) => void;
};

export const DatePickerComponent = ({
  label,
  onChange,
  ...datePickerProps
}: Props) => {
  const onChangeHandler = (
    date: Date | null,
    event?: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>
  ) => {
    onChange(date);
  };

  return (
    <label>
      <span>{label}</span>
      <DatePicker {...datePickerProps} onChange={onChangeHandler} />
    </label>
  );
};
arendora commented 5 days ago

I appreciate the suggestion @talatkuyuk and it is a temporary workaround, however that just overrides the issue and I think the underlying type change should still be addressed.

talatkuyuk commented 5 days ago

Thank you @arendora, but, I don't think it is a temporary workaround, it should be as I explained.

You pass a setState function to <DatePickerComponent /> as onChange prop, which is not an event handler for sure, and you shouldn't. I still think there is no issue about the type of onChange in the package. Sorry to say, you use the onChange prop in a wrong way.

If you want to pass strictly an appropriate event handler as a prop of DatePickerProps, define it in App.tsx, and pass it to the DatePicker component.

export default function App() {
  const [startDate, setStartDate] = useState<Date | null>(new Date());

  const onChange = (
    date: Date | null,
    event?: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>
  ) => {
    setStartDate(date);
  };

  return (
    <div className="App">
      <DatePickerComponent
        label="Date"
        onChange={onChange}
        // ...
      />
    </div>
  );
}

In that case, in the component you don't need to destruct onChange which will be in {...datePickerProps}:

type Props = DatePickerProps & {
  label: string;
};

export const DatePickerComponent = ({
  label,
  ...datePickerProps
}: Props) => {
  return (
    <label>
      <span>{label}</span>
      <DatePicker {...datePickerProps}  />
    </label>
  );
};

That is my conclusion.