Make it easier to replace Select/Dropdown components

[mihkeleidast]

Currently, Select/Dropdown expect onChange prop to be compatible with a native select, i.e. it expects a native event param. DayPicker component only reads e.target.value from that event here: https://github.com/gpbl/react-day-picker/blob/main/src/DayPicker.tsx#L231

If I want to replace the native select with a custom select component, say react-select, then most of such custom react select implementations do not provide event as a parameter in onChange, rather they forward only the selected value.

Since DayPicker does not need the actual event parameter, it would help if Dropdown/Select components had a custom onChange prop implementation that only requested the necessary value.

I can work around this by essentially only providing the required event object to the native onChange callback, but it's a bit hacky and not very nice to use:

onChange?.({
  target: {
    value: nextValue ?? '',
  },
} as React.ChangeEvent<HTMLSelectElement>);

[gpbl]

Oh I understand what you mean. I’ve found myself writing that casting as "hacky” solution as well.

Thinking how to ensure this new behavior to be backward compatible?

• have another prop, e.g. onChangeValue(value), to use instead
• have some TypeScript magic allowing the same callback handle different arguments types
• pass the value as second argument, e.g. onChange(e: ChangeEvent, value: string).
• just use { target: { value: string } } as type for the onChange argument, removing the need of casting to ChangeEvent.

[mihkeleidast]

I think having another onChangeValue prop would be the best of these options.

• The TS magic can get complex to understand and maintain, not sure if it's even possible unless I see an implementation :)
• Passing the value as second argument still requires the first argument to be present? Then I'd still have nothing to put there.
• The object type for argument can seem unnecessarily noisy when doing this, e.g. I'd immediately come and ask "why is it like this".

[mihkeleidast]

There are actually other prop types as well that need casting right now:

• aria-label says it can be undefined, but it's always defined. I also think this should be a regular label prop, whether it's used as aria-label, visible label on visually hidden in some other way is the component's implementation detail. Our Select component for example requires label prop, which can be visually hidden with another hideLabel prop.
• value says it can be undefined, but should exist
• options say they can be undefined, but IMO should always exist

[rodgobbi]

Thanks for the feedback @mihkeleidast
The issues you mentioned come from reusing the native HTMLSelectelement props, except options.
I already made a note to consider being more explicit about the prop types used by the sub-components, and your feedback helps clear up whether this would ultimately be helpful.
The main problem is that making types more restricted is a breaking change, be it removing a field from a type or making an optional field mandatory, so these changes could not be released in the current major version.

About

The TS magic can get complex to understand and maintain, not sure if it's even possible unless I see an implementation :)

There are a few approaches to extend the onChange type signature to allow it to accept the option value as direct argument,
one is function overloads, it would look as follows:

function onChange(value: string): void;
function onChange(e: ChangeEventHandler<HTMLSelectElement>): void;

The other option could be confusing to read, with a signature supporting both the event or the option value:

function onChange(eventOrValue: ChangeEventHandler<HTMLSelectElement> | string): void;

Both signatures would have the same implementation but read differently by the consumers of the lib.

[mihkeleidast]

Making an optional field mandatory is a breaking change if it affects consumers, but for these components the consumer is react-day-picker itself, so you can make the necessary changes (if any are even necessary) together with the prop type changes?

Not sure how removing "undefined" from these props can be breaking - for example, if your custom Select component today accepts value: string | number | undefined, then it will also accept value: string | number.

[rodgobbi]

but for these components the consumer is react-day-picker itself

The library uses the types and components but also exports the types, so any external consumer relying on them or implementing a custom component may have issues with a breaking change to a type.

Not sure how removing "undefined" from these props can be breaking - for example, if your custom Select component today accepts value: string | number | undefined, then it will also accept value: string | number

If any consumer is relying on value being possibly undefined, it would need to get updated to reflect the new type, which unfortunately even though it's a small change, it is still a breaking change.

undefined is especially tricky because the most common use case is to ignore if it's undefined, for example, any consumer doing options?.map(...) or value ?? 0 would not get affected by the breaking change, but if any consumer doing something like:

if (typeof value === 'undefined') {do X}
else {do Y}

X would never happen again thus being a breaking change.

But I also see the point for this specific case, as some properties may never have a value of undefined, and removing it from the type doesn't break any TS check.
If there's no situation where TS breaks with such type update I think it's possible to do it in the same major version.
We can try to check if this is possible.

[rodgobbi]

Hi @mihkeleidast and @gpbl

I was finishing writing this comment when @gpbl just posted the previous comment. 😅 I'll post in case it's helpful.

I understand the confusion, as the onChange prop is designed for the native select.

Fortunately, there's another API that I think is exactly what you need for your use case: the goToMonth function, which can be accessed through the DayPickerContext.
You can call it with the target month you want to display in the calendar after the user interacts with the custom Select dropdown.
You can check that it's called at the end of the handleMonthChange callback you shared. The callback has extra logic to set the correct month to call goToMonth, you need to implement something similar based on your use case to create the correct Date object to pass as the argument to goToMonth.

I'm reading the docs and it's not clear that the month parameter from goToMonth is of type Date, I think it would be clearer to explicitly express the type there.
Would this be helpful when reading the docs?

Also, I see that it's not clear how to implement a custom Select dropdown for the DayPicker, I think it's a good idea to add an example for it in the Custom Components guide.
Let me know if this would also be helpful.

[mihkeleidast]

Yeah, it's an alternative option, but ideally I would want to avoid duplicating the handleMonthChange logic, also handleYearChange as that'd be a custom Select as well. Not sure if Dropdown/Select components know which one it is (probably can deduce from passed options, but that's not ideal either).

[rodgobbi]

FYI, you differentiate by customizing MonthsDropdown or YearsDropdown, the Dropdown is rendered by both passing different props.