fix (types): onDateChange → onChange
#612
Conversation
BREAKING CHANGE: Our typedefs were exposing `onDateChange` (wrong signature) instead of `onChange` (correct signature). We replaced the `onDateChange` type with `onChange`.
| */ | ||
| onDateChange?(newDate: Date): void; | ||
| onChange?(newDate: Date): void; |
There was a problem hiding this comment.
This might be confusing.
The final type of your component props is defined as (214 line)
export type ReactNativeModalDateTimePickerProps = DateTimePickerProps &
Omit<IOSNativeProps, "value" | "mode"> &
Omit<AndroidNativeProps, "value" | "mode">;
IOSNativeProps and AndroidNativeProps are coming from @react-native-community/datetimepicker library. and
onChange?: (event: Event, date?: Date) => void; presents in both IOSNativeProps and AndroidNativeProps.
So the final type of the onChange will be
((date: Date) => void) & ((event: Event, date: Date) => void)
Are you sure you want to change types of the original library?
There was a problem hiding this comment.
Good catch — I added onChange to the omitted types. I'm not sure if that makes it less confusing to you, but we're already overriding other props (e.g., mode, value) with the ones we support.
As long as the prop is documented in the README.md, I think we should be fine (maybe we can update the README.md mentioning that we support only the non-overridden community-picker prop — but this hasn't been a problem so far).
...unless you think we should expose the event as well. I would prefer to not add additional complexity to the API we're exposing unless is needed, and I don't think the event has any meaningful info that couldn't already be obtained in other ways (but let me know if I'm missing something).
There was a problem hiding this comment.
Thanks for changing it.
Latest suggestion solves my problem.
I am not sure how meaningful event is. I cant' find use case for me, but some people are using it in the datetimepicker library: react-native-datetimepicker/datetimepicker#359
|
Thanks again @punksta for raising the issue and reviewing the PR! 🙌 |
# [13.0.0](v12.0.0...v13.0.0) (2021-10-25) * fix (types): `onDateChange` → `onChange` (#612) ([93cddd4](93cddd4)), closes [#612](#612) ### BREAKING CHANGES * Our typedefs were exposing `onDateChange` (wrong signature) instead of `onChange` (correct signature). We replaced the `onDateChange` type with `onChange`.
|
🎉 This PR is included in version 13.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Our typedefs were exposing
onDateChange(wrong signature) instead ofonChange(correct signature). We replaced theonDateChangetype withonChange.Fixes #611