types: DayPickerProps to use generic constraints

[ongyuxing]

Context

Fix issue #1583, where passing a union type to the mode prop of DayPicker results in a TypeScript warning that the type of mode is incorrect.

Analysis

TypeScript does not support merging property types from multiple interfaces in a union type.

Solution

I updated the DayPickerProps type definition to include a generic constraint. Additionally, I provided a default type for the generic parameter to avoid breaking changes.

[gpbl]

@ongyuxing this seems the solution we were looking for. Thanks for the help! I'll check the tests which have too much logs to be properly debugged...

[ongyuxing]

@gpbl I tried to fix the issue that onSelect parameter inference is incorrect when mode is a union type by converting onSelect to an intersection type. But it requires using a utility types library called type-fest that includes a UnionToIntersection type. Would you recommend using this library and type for my purposes?

[gpbl]

Would you recommend using this library and type for my purposes?

@ongyuxing I'm interested why type-fest would be necessary? The update you propose seems to work. (The failing test is unrelated to this change.)

Also could we add an example file where we see this change working as intended? Like in website/examples.

Thanks!

[ongyuxing]

@gpbl When the mode type is a union of 'single' | 'multiple', the onSelect type will also be inferred as a union of SelectSingleEventHandler | SelectMultipleEventHandler. This will result in a TypeScript error when passing in onSelect.

To fix this error, the type of onSelect needs to be converted to SelectSingleEventHandler & SelectMultipleEventHandler, allowing TypeScript to correctly merge the types of parameters and return values. For example, the first parameter will be inferred as a union of Date | Date[].

This is an example:

import type {
  DayPickerBase,
  DayPickerDefaultProps,
  DayPickerMultipleProps,
  DayPickerRangeProps,
  DayPickerSingleProps,
  DaySelectionMode,
  SelectMultipleEventHandler,
  SelectRangeEventHandler,
  SelectSingleEventHandler,
} from 'react-day-picker'
import type { UnionToIntersection } from 'type-fest'

interface DayPickerProps<T extends DaySelectionMode = DaySelectionMode>
  extends DayPickerBase {
  mode?: T
  selected?: T extends 'single'
    ? DayPickerSingleProps['selected']
    : T extends 'multiple'
    ? DayPickerMultipleProps['selected']
    : T extends 'range'
    ? DayPickerRangeProps['selected']
    : DayPickerDefaultProps['selected']
  onSelect?: SelectEventHandler<T>
  required?: T extends 'single' ? DayPickerSingleProps['required'] : never
  min?: T extends 'multiple'
    ? DayPickerMultipleProps['min']
    : T extends 'range'
    ? DayPickerRangeProps['min']
    : never
  max?: T extends 'multiple'
    ? DayPickerMultipleProps['max']
    : T extends 'range'
    ? DayPickerRangeProps['max']
    : never
}

type SelectEventHandler<T extends DaySelectionMode = DaySelectionMode> =
  T extends 'single'
    ? SelectSingleEventHandler
    : T extends 'multiple'
    ? SelectMultipleEventHandler
    : T extends 'range'
    ? SelectRangeEventHandler
    : never

declare function DayPicker<T extends DaySelectionMode = 'default'>(
  props: Omit<DayPickerProps<T>, 'onSelect'> & {
    // Directly changing onSelect in DayPickerProps is not recommended as it will break internal references.
    onSelect: UnionToIntersection<SelectEventHandler>
  }
): JSX.Element

const DatePicker = ({ index }: { index: number }) => {
  const modes = ['single', 'multiple', 'range'] as const
  const values = [
    new Date(),
    [new Date()],
    { from: new Date(), to: new Date() },
  ]

  return (
    <DayPicker
      mode={modes[index]}
      selected={values[index]}
      onSelect={(value) => {}}
    />
  )
}

[ongyuxing]

This is the running result without any modification: This is the running result without modifying the type of onSelect: This is the running result after the modification:

[gpbl]

@ongyuxing I see it now – thanks for the clear explanation! So yes, feel free to add the type-fest dependency. This is a great change, looking forward to release it 👍

[ongyuxing]

@gpbl It is done.🎉

[gpbl]

Thanks @ongyuxing . I'm seeing interesting types errors... I am looking at them.

[gpbl]

Sorry for hi-jacking your PR, I couldn't get actions working on your fork... I added a test and removed the need of type-fest in 6571670 (#1718).

Could we start from this change? What is missing in the test to require the type-fest utility?

[ongyuxing]

@gpbl The argument for onSelect is currently being inferred as any type. You need to add "strict: true" to the "compilerOptions" in the tsconfig.json for website. Then TypeScript will report an error.

[gpbl]

@gpbl The argument for onSelect is currently being inferred as any type. You need to add "strict: true" to the "compilerOptions" in the tsconfig.json for website. Then TypeScript will report an error.

LetS investigate this in another PR. I think it will fail for other cases too, not related to this change. I remember we tried to set strict mode and it wasn't working well. I cannot find the PR right now.

[gpbl]

OK I want to give it a second try :) I'm investigating these errors in the test: https://github.com/gpbl/react-day-picker/actions/runs/4387214125/jobs/7824904268#step:6:20

[gpbl]

If I make the onSelect optional, without strict mode the only thing that is failing is:

export default function Example(props: DayPickerProps) {
  return <DayPicker {...props} />;
}

I don't think we can make much progress from here. In the upcoming version we should rethink the component prop considering this case.

[ongyuxing]

@gpbl DayPickerProps doesn't match the actual DayPicker props, but changing it will break internal references. A less elegant solution is adding a standalone DayPickerProps type for internal use.

I don't think we can make much progress from here. In the upcoming version we should rethink the component prop considering this case.

You're right! If the onSelect type is modified like this:

type SelectEventHandler = <
  T extends DaySelectionMode = DaySelectionMode
>(
  values: {
    day?: T extends 'single' ? Date : never;
    days?: T extends 'multiple' ? Date[] : never;
    range?: T extends 'range' ? DateRange : never;
    selectedDay: Date;
    activeModifiers: ActiveModifiers;
  },
  e: React.MouseEvent
) => void;

It will make writing type definitions much easier.

[ongyuxing]

Just as an experiment, I added an InternalDayPickerProps type.

[balusio]

Hello, i'm facing this issue too with dayPicker, do we have any ideas when can have this merged?

[samuelg0rd0n]

Any updates on this please?

[gpbl]

@samuelg0rd0n we plan to implement this in the next mayor version (still WIP), as it may be a breaking change.

[gpbl]

Please checkout this proposal for generic types in DayPickerProps.

Any feedback?

[AntoineKM]

Any news here ?

[XXXVII]

Any updates on this one?