mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[pickers] `DateCalendar`'s `onChange` prop has `any` argument type instead of `PickerValidDate`

Open vrama628 opened this issue 1 year ago • 10 comments

Steps to reproduce

Link to live example: https://stackblitz.com/edit/react-wjvuys?file=Demo.tsx

Steps:

  1. Import AdapterDayjs date adapter as described in https://mui.com/x/react-date-pickers/base-concepts/#typing-of-the-date
  2. Create component with LocalizationProvider with the AdapterDayjs date adapter, and a DateCalendar with an onChange prop
  3. hover over the onChange prop to observe that the argument type is any instead of using the type of PickerValidDate

Current behavior

the argument type of onChange is any

Expected behavior

the argument type of onChange should be dayjs.Dayjs

Context

I'm trying to use DateCalendar in TypeScript, but even when using the recommended pattern for type safety, the DateCalendar library explicitly uses any to opt out of type checking. This problem does not seem to happen with other props such as value; it seems to be specific to how the types are implemented for onChange.

I believe this bug is caused by the ExportedUseViewsOptions type that DateCalendar uses for its props type. It seems to explicitly set UseViewsOptions' TValue type to any instead of using the TDate/PickerValidDate type as DateCalendar's other props do. link: https://github.com/mui/mui-x/blob/4c6b35fa8d8d53231c1524fa6fae8194bb9a9e76/packages/x-date-pickers/src/internals/hooks/useViews.tsx#L66-L67

Note: in the reproduction link above, there PickerValidDate also ends up with type any despite using the recommended pattern for setting it. This is likely an unrelated bug, as locally my PickerValidDate is receiving the correct dayjs.Dayjs -- but even then, the onChange prop has any argument type.

Your environment

npx @mui/envinfo
  System:
    OS: macOS 13.5
  Binaries:
    Node: 22.6.0 - /opt/homebrew/bin/node
    npm: 10.8.2 - /opt/homebrew/bin/npm
    pnpm: 9.1.4 - ~/Library/pnpm/pnpm
  Browsers:
    Chrome: 130.0.6723.70
    Edge: Not Found
    Safari: 16.6
  npmPackages:
    @emotion/react: ^11.13.0 => 11.13.0 
    @emotion/styled: ^11.13.0 => 11.13.0 
    @mui/core-downloads-tracker:  5.16.6 
    @mui/icons-material: ^5.16.6 => 5.16.6 
    @mui/material: ^5.16.6 => 5.16.6 
    @mui/private-theming:  5.16.6 
    @mui/styled-engine:  5.16.6 
    @mui/system:  5.16.6 
    @mui/types:  7.2.15 
    @mui/utils:  5.16.6 
    @mui/x-date-pickers: ^7.22.0 => 7.22.0 
    @mui/x-internals:  7.21.0 
    @types/react: ^18 => 18.3.3 
    react: ^18 => 18.3.1 
    react-dom: ^18 => 18.3.1 
    typescript: ^5 => 5.5.4 

Search keywords: date, datecalendar, datepicker, calendar, picker, typescript, pickervaliddate, any

vrama628 avatar Oct 27 '24 20:10 vrama628

Thanks for opening an issue here @vrama628 ... it seems we could possibly adjust this to use the correct type.

@flaviendelangle should we add this to the board for now?

michelengelen avatar Oct 29 '24 11:10 michelengelen

I'll have a look, the type should be correctly defined :grimacing:

flaviendelangle avatar Oct 29 '24 11:10 flaviendelangle

@vrama628 do you have the problem on an actual application or only on Stackblitz? Because I have the problem on Stackblitz, but if I clone the repo and open in VSCode then it goes away...

flaviendelangle avatar Oct 29 '24 12:10 flaviendelangle

@vrama628 Here is an updated StackBlitz example with needed fixes to avoid the incorrect any resolution. The problem was mainly with the mix of Date and Dayjs types. 😉 Does it resolve your issue?

LukasTy avatar Oct 29 '24 12:10 LukasTy

@LukasTy the problem is that TS did not complain in his example :grimacing:

flaviendelangle avatar Oct 29 '24 12:10 flaviendelangle

the problem is that TS did not complain in his example 😬

Oh, gotcha. 👍 Sorry for missing the point. 🙈 Regardless, TS reports a problem with the provided demo locally. Do we need to fixate on understanding the infra behind sandboxes and why their tsc is providing different results? 🤔

LukasTy avatar Oct 29 '24 14:10 LukasTy

As I understand it, the codesandbox is here to demonstrate that PickerValidDate is any| instead of Dayjs.

flaviendelangle avatar Oct 29 '24 15:10 flaviendelangle

As I understand it, the codesandbox is here to demonstrate that PickerValidDate is any| instead of Dayjs.

Did you mean never, instead of Dayjs? 🤔 💡 This is a screenshot of the example replicated in an isolated project using only AdapterDayjs. Screenshot 2024-10-29 at 17 56 51

LukasTy avatar Oct 29 '24 15:10 LukasTy

:thinking: in his example he has any, he should have Dayjs since he imports AdapterDayjs And locally I have Dayjs, not sure how you ended up with never here :grimacing:

flaviendelangle avatar Oct 29 '24 16:10 flaviendelangle

🤔 in his example he has any, he should have Dayjs since he imports AdapterDayjs And locally I have Dayjs, not sure how you ended up with never here 😬

Sorry, scratch that. 🙈 I missed the fact that v7.14.0 was installed. 🤦 It is indeed dayjs.Dayjs after updating to the latest package version. 👌

LukasTy avatar Oct 29 '24 16:10 LukasTy

The issue has been inactive for 7 days and has been automatically closed.

github-actions[bot] avatar Nov 05 '24 15:11 github-actions[bot]