date-io icon indicating copy to clipboard operation
date-io copied to clipboard

strict parsing a date in dayjs by default

Open hamcher opened this issue 3 years ago • 7 comments

Hi, we found a problem when we using AdapterDayjs with mui date picker in the validation

hamcher avatar Nov 30 '21 15:11 hamcher

How this will imply on the previous working examples? We must be backward compatible

dmtrKovalenko avatar Nov 30 '21 15:11 dmtrKovalenko

Codecov Report

Merging #599 (2c43bd1) into master (e70582f) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #599   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        16           
  Lines         1460      1460           
  Branches       198       197    -1     
=========================================
  Hits          1460      1460           
Impacted Files Coverage Δ
packages/dayjs/src/dayjs-utils.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e70582f...2c43bd1. Read the comment docs.

codecov-commenter avatar Nov 30 '21 15:11 codecov-commenter

How this will imply on the previous working examples? We must be backward compatible

It will be a breaking change I guess as without strict parsing, dayjs will consider invalid dates to be valid. For example, 03/13/2021 (as DD/MM/YYYY, but the same will apply to any other format) without strict mode will parse as 03/01/2022, wrapping the months. Another example is that 78/56/1234 parses as the ISO string 1238-10-16T22:47:48.000Z. With strict mode, both of these will return an Invalid Date, which is more logical and more likely what is expected.

This is documented on the website: https://day.js.org/docs/en/parse/string-format

chwallen avatar Feb 07 '22 00:02 chwallen

The biggest question from me – will it require breaking change? If not - I will be happy to merge it and release right now

dmtrKovalenko avatar Feb 16 '22 19:02 dmtrKovalenko

The biggest question from me – will it require breaking change? If not - I will be happy to merge it and release right now

I cannot answer more than I did above. Without strict mode, dayjs will turn invalid dates valid. With it, invalid dates will be returned as invalid. If you consider this breaking, then sure.

chwallen avatar Feb 16 '22 21:02 chwallen

Can you guys merge either #599 OR #608 ? Changes are equal... but the fix would be great... :grimacing: @dmtrKovalenko

kjellski avatar May 05 '22 09:05 kjellski

As the other PR was merged, this can be closed :)

kjellski avatar May 05 '22 11:05 kjellski