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] Dayjs adapter gives unexpected results for invalid date inputs #6983

Closed
2 tasks done
tetchel opened this issue Feb 3, 2022 · 4 comments
Closed
2 tasks done
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module!

Comments

@tetchel
Copy link

tetchel commented Feb 3, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

This is the same issue as @RCout1nho in https://github.com/mui-org/material-ui/issues/29838, however I felt like this deserved its own issue because it's with the DatePicker component, not the TimePicker.

If you input an invalid date into the MUI DatePicker using the dayjs adapter, many dates are accepted as valid that shouldn't be.
For example, inputting the month as "13" and having it wrap to January of the next year seems incorrect to me.
Since MUI is doing the dayjs call internally, the library user cannot work around this.

As mentioned in that issue using the strict arg is probably the right solution as per iamkun/dayjs#320

I am going to have to switch to using the luxon or date-fns adapter which is too bad because they are much larger libraries.

Expected behavior 🤔

MUI should provide an error message when an invalid date is input.

OR at least provide some escape hatch for the library user to get the raw user input in onChange so the library user can manage the dayjs calls by hand.

Steps to reproduce 🕹

Steps:

https://codesandbox.io/s/basictimepicker-material-demo-forked-1hflu

  1. Input the same invalid date into both textboxes. Use a date that "overflows" the maximum number of days/months, eg 32 days or 13 months. The result is the same if eg. inputting 02/29 for a year that didn't have a leap year eg 02/29/2022.
  2. Note that the date-fns one is 'invalid', but the dayjs one coerces it to a valid date.

image
image

Context 🔦

I want my user to only be able to input dates that make sense.

Your environment 🌎

`npx @mui/envinfo`

Browser is chromium

  System:
    OS: Linux 5.15 Fedora Linux 35 (Workstation Edition)
  Binaries:
    Node: 16.13.2 - /usr/bin/node
    Yarn: 1.22.17 - /usr/local/bin/yarn
    npm: 8.1.2 - /usr/bin/npm
  Browsers:
    Chrome: Not Found
    Firefox: 96.0
  npmPackages:
    @emotion/react: 11.7.1 => 11.7.1
    @emotion/styled: 11.6.0 => 11.6.0
    @mui/base:  5.0.0-alpha.64
    @mui/icons-material: 5.2.5 => 5.2.5
    @mui/lab: 5.0.0-alpha.64 => 5.0.0-alpha.64
    @mui/material: 5.2.8 => 5.2.8
    @mui/private-theming:  5.2.3
    @mui/styled-engine:  5.2.6
    @mui/system:  5.2.8
    @mui/types:  7.1.0
    @mui/utils:  5.2.3
    @types/react: 17.0.38 => 17.0.38
    react: 17.0.2 => 17.0.2
    react-dom: 17.0.2 => 17.0.2
    typescript: 4.5.4 => 4.5.4
@tetchel tetchel added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 3, 2022
@danilo-leal danilo-leal removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 4, 2022
@RCout1nho
Copy link

RCout1nho commented Feb 14, 2022

I think the error is actually in the date-io/dayjs lib. Material-ui's AdapterDayJs (@mui/lab/AdapterDayJs) is just a wrapper for @date-io/dayjs.
In the parse method, dayjs needs the strict param as true, or it will convert an invalid date to a valid one just by summing the days that have passed, for example: "99/99/2000", if the parameter strict is false (default value), the dayjs will add (99-12) months + (99-30) days to 12/12/2000 and would validate the result.
Also, this.locale is typed incorrectly, since for dayjs this attribute is of type ILocale, not string.
I am finishing a correction attempt and I will open a PR in the date-io repository

@RCout1nho
Copy link

I finished the fix and opened a PR. While PR is not accepted, if you want, you can use temporarily this way:

  1. Download this folder and put anywhere in your project
  2. Instead of using @mui/lab/AdapterDayjs, use the imported folder (dayjs/src)

@selfrefactor
Copy link

any progress as it seems like the issue is stuck without progress for too long? Proposal from @RCout1nho works great and I am thankful. Still, I don't like to have a library file in my source code.

@flaviendelangle flaviendelangle transferred this issue from mui/material-ui Nov 24, 2022
@flaviendelangle flaviendelangle changed the title [Datepicker] Dayjs adapter gives unexpected results for invalid date inputs [pickers] Dayjs adapter gives unexpected results for invalid date inputs Nov 24, 2022
@flaviendelangle flaviendelangle changed the title [pickers] Dayjs adapter gives unexpected results for invalid date inputs [DatePicker] Dayjs adapter gives unexpected results for invalid date inputs Nov 24, 2022
@flaviendelangle
Copy link
Member

The last version of the library no longer has the bug see this reproduction case

image

@flaviendelangle flaviendelangle added bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! labels Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

5 participants