mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.16k stars 1.3k forks source link

[pickers] `AdapterDateFns` fires `onChange` on invalid dates (with year `1`) #6716

Open githorse opened 1 year ago

githorse commented 1 year ago

Duplicates

Latest version

Steps to reproduce 🕹

See stackblitz demo.

Current behavior 😯

With AdapterDateFns and an input format of yyyy-MM-dd, the onChange event fires after typing 2022-02-2 (without hitting enter) with a date value of 2022-02-02. With an input format of dd/MM/yyyy, it fires on 12/12/1 with a date value of December 12, 0001 (which I just learned is possible!). Interestingly, the date picker itself knows that these dates are invalid, since it outlines the inputs in red:

image

Other adapters (e.g. AdapterDayjs) are much better behaved and seem to fire only on things a reasonable person would consider valid dates.

Of course, it's very possible the real issue here is the underlying behavior of date-fns and not the adapter per se. At the moment, though, it's not clear to me how anyone can use this adapter with all the spurious onChange events happening as the user edits the field.

Expected behavior 🤔

The component should wait to fire onChange until I've entered a valid date according to the mask format: 2022-02-20 (like other adapters). At minimum, the behavior should be consistent across different adapters.

Note that this ask is sort of the opposite of https://github.com/mui/mui-x/issues/5755. It's not immediately clear how to reconcile these so that it works intuitively, but I would argue 2022-02-2 and 12/12/1 don't look like valid dates and shouldn't be interpreted as such.

Context 🔦

Prematurely firing onChange caused a bug in my app where spurious action is taken on the onChange event, resulting in users being unable to type 2022-02-20 because the date gets set to 2022-02-02. On many pages, I make an API call when the onChange event updates the date -- obviously I can't be doing that on intermediate, incorrect values as the user edits the field.

No matter what input format I use, I get all kinds of spurious onChange events while the user is typing stuff in the field. I will likely have to switch to a different adapter to get around the issue, but this is a non-trivial refactor as AdapterDateFns seems to be the only one that works on a native Javascript Date object, and changing that breaks a lot of code.

Your environment 🌎

npx @mui/envinfo ``` System: OS: Linux 5.4 Linux Mint 20.3 (Una) CPU: (8) x64 Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz Memory: 954.71 MB / 15.33 GB Container: Yes Shell: 5.8 - /usr/bin/zsh Binaries: Node: 16.15.0 - ~/.nvm/versions/node/v16.15.0/bin/node Yarn: 3.2.4 - ~/.yarn/bin/yarn npm: 8.5.5 - ~/.nvm/versions/node/v16.15.0/bin/npm Watchman: 20210409.063814.0 - /usr/local/bin/watchman Managers: Cargo: 1.51.0 - ~/.cargo/bin/cargo pip3: 20.0.2 - /usr/bin/pip3 RubyGems: 3.1.2 - /usr/bin/gem Utilities: CMake: 3.16.3 - /usr/bin/cmake Make: 4.2.1 - /usr/bin/make GCC: 9.4.0 - /usr/bin/gcc Git: 2.25.1 - /usr/bin/git FFmpeg: 4.2.7 - /usr/bin/ffmpeg Virtualization: Docker: 20.10.21 - /usr/bin/docker IDEs: Nano: 4.8 - /usr/bin/nano Vim: 0.4.3 - /usr/bin/vim Languages: Bash: 5.0.17 - /usr/bin/bash Java: 11.0.11 - /home/rct/.sdkman/candidates/java/current/bin/javac Perl: 5.30.0 - /usr/bin/perl Protoc: 3.17.0 - /usr/local/bin/protoc Python3: 3.8.10 - /usr/bin/python3 Ruby: 2.7.0 - /usr/bin/ruby Rust: 1.51.0 - /home/rct/.cargo/bin/rustc Scala: 2.12.12 - /home/rct/.sdkman/candidates/scala/current/bin/scalac Databases: SQLite: 3.32.2 - /home/rct/Android/Sdk/platform-tools/sqlite3 Browsers: * Chromium: 106.0.5249.119 Firefox: 106.0.3 Monorepos: Yarn Workspaces: 3.2.4 Lerna: 5.6.2 ```

Order ID 💳 (optional)

46840

flaviendelangle commented 1 year ago

Interestingly, the date picker itself knows that these dates are invalid, since it outlines the inputs in red:

The pickers have a default minDate set to 1970, this is probably why you have your date in red. In the next major, we are going to pass the validation result of the date on onChange to help people ignore the invalid one if they want, this should solve your use case.


2022-02-2 is a valid date for the format YYYY-DD-M on dayjs (which I do agree is probably not used a lot).

As you pointed out, this difference in behavior comes from the way the various date libraries parses the date and consider them to be valid or not.

We try not to override these behaviors because it risk to cause lots of bugs with date format we don't know. I think passing the validation error and letter people ignore those events is a good middle ground here.

githorse commented 1 year ago

After playing with them a bit, I've found that AdapterMoment, AdapterDayjs, and AdapterLuxon all seem to do what I expect here and only fire onChange on what I intuitively consider to be a valid date, no matter the input format. (I'm fine with 2022-02-2 firing on input format YYYY-DD-M; I'm not fine with it firing on YYYY-MM-DD.) Only AdapterDateFns seems to be so cavalier about what it considers a valid date.

In the next major, we are going to pass the validation result of the date on onChange to help people ignore the invalid one if they want, this should solve your use case.

To me it seems a bit weird and confusing to have two different sources of truth here -- one from the adapter and one from the component. It would be nice if the DatePicker would just correctly give me a date, or null (which is exactly what happens with the adapters apart from AdapterDateFns). I do see your issue with supporting strange date formats. I kind of think those could be unified as well, but I suppose it might be impossible to capture all possible international formats somebody might want to support. Tough problem!

Clearly the root of my issue here is that date-fns is just way too lax about what it considers valid dates. My guess here is this is just using native Javascript Date.parse blindly without looking at the inputFormat? Though this doesn't get me the 0001 year -- I don't know where that comes from. At any rate Date.parse does not accept an input format string, so can't help me here because it's also fine with 2022-02-2.

I was able to solve my problem by doing my own check on the raw keyboard input value:

const PARSER = {
  format: "yyyy-MM-dd",
  mask:   "____-__-__",
  regex:  /^(\d{4})-(\d{2})-(\d{2})$/
}

...

const handleChange = useCallback(
  (date: Date | null, rawInput?: string) => {
    if (!date || !rawInput || !rawInput?.length) { return }
    if (!PARSE.regex.test(rawInput)) { return }
    onChange(date)
  },
  []
)

return (
  <DatePicker
    inputFormat={PARSER.format}
    mask={PARSER.mask}
    onChange={handleChange}
    ...
/>

Too bad I need to define three redundant pieces of information here, but this is good enough to unblock me.

flaviendelangle commented 1 year ago

To me it seems a bit weird and confusing to have two different sources of truth here -- one from the adapter and one from the component.

You don't have 2 sources of truth The adapter tells you if the date is a valid date The component tells you if the date passes the validation (minDate / maxDate / shouldDisableDate etc...)

On dayjs I know that there is a "strict" parsing param to prevent those kind of date to be considered valid. Maybe date-fns has one as well, we can have a look and if there is enable it in the adapter.