New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
types: DayPickerProps to use generic constraints #1718
base: main
Are you sure you want to change the base?
Conversation
@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... |
@gpbl I tried to fix the issue that |
@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! |
@gpbl When the To fix this error, the type of 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 I see it now – thanks for the clear explanation! So yes, feel free to add the |
@gpbl It is done.🎉 |
Thanks @ongyuxing . I'm seeing interesting types errors... I am looking at them. |
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 Could we start from this change? What is missing in the test to require the type-fest utility? |
@gpbl The argument for |
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. |
OK I want to give it a second try :) |
If I make the 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. |
@gpbl
You're right! If the 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. |
Just as an experiment, I added an InternalDayPickerProps type. |
Hello, i'm facing this issue too with dayPicker, do we have any ideas when can have this merged? |
Any updates on this please? |
@samuelg0rd0n we plan to implement this in the next mayor version (still WIP), as it may be a breaking change. |
Please checkout this proposal for generic types in Any feedback? |
Any news here ? |
Any updates on this one? |
Context
Fix issue #1583, where passing a union type to the
mode
prop ofDayPicker
results in a TypeScript warning that the type ofmode
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.