react-datepicker icon indicating copy to clipboard operation
react-datepicker copied to clipboard

fix: Inconsistent/broken behavior in `parseDate`

Open maranomynet opened this issue 2 years ago • 5 comments

Note This is a re-opening of #3903 (since reverted by #3976) in the hope of it eventually getting accepted as a "breaking change".

See the expanded commit messages for details.

After this fix, the parsing behavior of react-datepicker

a) is more consistent between locales/browsers/OS platforms b) has fewer bugs c) is the same whether dateFormat is string or an array of strings d) treats arrayed dateFormat values as "most preferred first" (instead of the opposite). e) is slightly more "strict".

Item "e" results from parseDate no longer falling back (with string values for props.dateFormat) on free-form, hard-to-predict new Date(value) parsing of input values when date-fns has already declared the value as "invalid".

It seems sensible (and more deterministic) to place the responsibility of parsing fully on date-fns and not (sometimes) second-guesss its results.

The new Date(value) parsing causes surprising and deceptively incorrect "false positive" parsing in situations where the dateFormat was dd.MM.yyyy and users type in a date in this format dd.MM.yy. Then their input gets "helpfully" re-parsed as MM.dd.yy, which was most definitely not intended by the developer setting up the datepicker input.

However, there's still a lot of flexibility in the parsing, as date-fns/parse provides very sensible flexibility based on localized RegExp configs, and also since props.dateFormat can be an Array of valid formattings.

As people are often relying on the old mis-behavior this PR needs to become part of a v5 release. A migration guide (and, possibly, updated documentation) could explain how to get the old behavior back by using props.dateFormat with multiple formats in order of decreasing priority.

maranomynet avatar Apr 06 '23 14:04 maranomynet

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.09%. Comparing base (716c0cb) to head (422bbbc). Report is 914 commits behind head on main.

:exclamation: Current head 422bbbc differs from pull request most recent head 59658cd. Consider uploading reports for the commit 59658cd to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3988      +/-   ##
==========================================
- Coverage   94.21%   94.09%   -0.13%     
==========================================
  Files          20       20              
  Lines        1746     1726      -20     
  Branches      419      419              
==========================================
- Hits         1645     1624      -21     
- Misses         30       31       +1     
  Partials       71       71              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 06 '23 14:04 codecov[bot]

It's been a long time since this was first submitted, but I agree with the thinking here. If you're still willing to bring this forward we can work towards releasing this as part of a major version bump.

martijnrusschen avatar Mar 10 '24 19:03 martijnrusschen

@maranomynet hello Can you update your PR based on this one https://github.com/Hacker0x01/react-datepicker/pull/4707 and then we can do a major release, including your breaking changes

mirus-ua avatar Apr 22 '24 19:04 mirus-ua

@martijnrusschen hello this PR looks promising. Should we try to adapt it for v7 with the rest of the breaking changes?

mirus-ua avatar May 04 '24 07:05 mirus-ua

@mirus-ua I think that would be good.

martijnrusschen avatar Jun 11 '24 12:06 martijnrusschen

It seems this PR was skipped over for v7, is there any chance of getting it included in a future v8? I would like to work on fixing #3596 #3906 #4002 and this patch would be a prerequisite for fixing the parsing of ranges.

laug avatar Aug 15 '24 09:08 laug

closing in favor of https://github.com/Hacker0x01/react-datepicker/pull/5036

martijnrusschen avatar Aug 28 '24 17:08 martijnrusschen