Skip to content
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

DatePicker and DateRangePicker onChange parameter type should include null. #6339

Closed
adriantrunzo opened this issue May 7, 2024 · 1 comment

Comments

@adriantrunzo
Copy link

adriantrunzo commented May 7, 2024

Provide a general summary of the issue here

The types for DatePicker and DateRangePicker are created with ValueBase<T | null, MappedDateValue<T>> (or ValueBase<RangeValue<T>, RangeValue<MappedDateValue<T>>>), where T extends DateValue, which means the onChange handler is typed as onChange?: (value: MappedDateValue<T>) => void. The MappedDateValue type is non-nullable, but it's trivial to cause these components to call onChange with a null. The type of the onChange parameter should be nullable.

🤔 Expected Behavior?

The exported type of onChange for DatePicker is (value: MappedDateValue<T> | null) => void and for DateRangePicker (value: RangeValue<MappedDateValue<T>> | null) => void.

😯 Current Behavior

The onChange parameter isn't typed as nullable, so TypeScript fails to alert users to a possible TypeError exception (eg due to de-structuring null).

💁 Possible Solution

Update the types for DatePicker, DateRangePicker, etc. to include null for the second generic parameter to ValueBase. For example:

export interface DatePickerProps<T extends DateValue> extends DatePickerBase<T>, ValueBase<T | null, MappedDateValue<T> | null> {}

What's the use case for MappedDateValue? I haven't found a reason why the type can't be simplified to just:

export interface DatePickerProps<T extends DateValue> extends DatePickerBase<T>, ValueBase<T | null> {}

🔦 Context

While creating a custom component composing DateRangePicker, I created an onChange handler similar to the following:

<DateRangePicker
  onChange={({ start, end }) => ...}
  {...others}
/>

I was surprised to get no error from TypeScript about the argument potentially being null. Here, a TypeError is thrown at runtime when trying to destructure null.

🖥️ Steps to Reproduce

Sandbox: https://codesandbox.io/p/sandbox/long-surf-m5zd6x

  • Focus on one of the date segments
  • Hit the Backspace key until you delete the segment value
  • See that the displayed value is null
Screen.Recording.2024-05-06.at.9.26.53.PM.mov

I imagine the "Clear" button example in the documentation will also cause onChange to be called with null.

Version

1.2.0

What browsers are you seeing the problem on?

Firefox, Chrome, Safari

If other, please specify.

No response

What operating system are you using?

mac OS 14.3

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

@snowystinger
Copy link
Member

Thanks for the issue, I'm going to close it as a duplicate. Should be fixed by #6076 and falls under the umbrella Strict mode issue #3183

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants