dmtrKovalenko / date-io

Abstraction over common javascript date management libraries

MIT License

727 stars 90 forks source link

strict parsing a date in dayjs by default #599

Closed hamcher closed 2 years ago

hamcher commented 2 years ago

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

dmtrKovalenko commented 2 years ago

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

codecov-commenter commented 2 years ago

Codecov Report

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

@@            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.

chwallen commented 2 years ago

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

dmtrKovalenko commented 2 years ago

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

chwallen commented 2 years ago

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.

kjellski commented 2 years ago

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

kjellski commented 2 years ago

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