dayjs

dayjs copied to clipboard

The issue #1596 shows that the plugin 'customParseFormat' does not give a valid date, when used with utc parsing dates with offset. I identified 2 interdependant reasons for this:

1. customParseFormat.parse gives wrong arguments to dayjs when called by utc plugin with an array of formats.
   Details:
   calling utc(...) for parsing the utc plugin creates a cfg object with a correct 'utc' property ('true'); but the recursion in the parse function of 'customParseFormat' just transfers all the arguments to the call to 'dayjs' (const result = d.apply(this, args)). But the args property must be a cfg object.

2. parsing a date with offset in strict mode always returns 'Invalid Date'.
   Details:
   (isStrict && date != this.format(format)) is an elegant way to find out, if the parsed date respects all the separators as defined in the format. But it fails, as soon as an offest comes into play; in this case the result of formatting the parsed date corresponds to the given format, but the used offset depends an the current local timezone and is not necessarily the same as the offset in the date to be parsed (e.g. '11:00+01' is the same as '12:00+02'). As a result I had to change the way we identify a 'strict' match (including the handling of overflows in strict mode).

I did not squash the single commits, so that it is easier to analyze the single modifications.
The replacement for (isStrict && date != this.format(format)) (e.g. the part 'with offset' of the issue #1596) is in commit 69fdc94 and 158aed8; the commits 963b166 and 6a2974e just handle the 'with utc' part of issue #1596) and 52902e0 the self reference.
The rest are just tests for all the affected parsing scenarios.

Perhaps we should add a list of the allowed separator characters (.-_:/() and any whitespace) to the customParseFormat plugin documentation ('https://day.js.org/docs/en/parse/string-format')? PR #1913 would add a comma to that list.

And when we are at it, perhaps we should also add a list of the deviations from moment to the documentation:

title	parameters	dayjs	moment
invalid date with overflow	('35/22/2010 99:88:77', 'DD-MM-YYYY HH:mm:ss')	'08-11-2011 04:29:17'	'Invalid date'
invalid date with overflow, strict	('35/22/2010 99:88:77', 'DD-MM-YYYY HH:mm:ss', true)	'Invalid Date'	'Invalid date'
'0' day or month (using default values)	('1970-00-00', 'YYYY-MM-DD')	'1970-01-01'	'Invalid date'
'0' day or month (using default values), strict	('1970-00-00', 'YYYY-MM-DD', true)	'Invalid Date'	'Invalid date'
date not matching format	('10/12/2014', 'YYYY-MM-DD')	'01-01-2014'	'12-20-2010'
date not matching format, strict	('10/12/2014', 'YYYY-MM-DD', true)	'Invalid Date'	'Invalid date'
first match vs. longest match	('2012-05-28 10:21:15', ['YYYY', 'YYYY-MM-DD', 'YYYY-MM-DD HH:mm:ss'])	'2012-01-01 00:00:00'	'2012-05-28 10:21:15'
first match vs. longest match, strict	('2012-05-28 10:21:15', ['YYYY', 'YYYY-MM-DD', 'YYYY-MM-DD HH:mm:ss'], true)	'2012-05-28 10:21:15'	'2012-05-28 10:21:15'

Fixes issue #1331 too. Fixes issue #1552 too. Fixes issue #1616 too. Fixes issue #1750 too. Fixes issue #1902 too.

I finally found the "older" pr #1467 that solves at least the issue with the offset in strict mode, when the format contains a Z token.

• the pr #1467 takes the approach of improving the way to identify a 'strict match' ('date != this.format(format)') while
• this pr tries to identify 'strictness' while parsing the date parts; this method allows to fix other issues with parsing that occurred e.g. when the sequence of date parts did not match the sequence of format token e.a.

So now you have the choice :-)

@iamkun by now there are a lot of open issues and quite some pr. Is there a way I could help you to reduce the number of open issues and pr?

Codecov Report

Merging #1914 (9bf4076) into dev (7151139) will not change coverage. The diff coverage is 100.00%.

:exclamation: Current head 9bf4076 differs from pull request most recent head af75753. Consider uploading reports for the commit af75753 to get more accurate results

@@            Coverage Diff            @@
##               dev     #1914   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          181       184    +3     
  Lines         2074      2158   +84     
  Branches       544       579   +35     
=========================================
+ Hits          2074      2158   +84     

Impacted Files	Coverage Δ
src/locale/fa.js	100.00% <ø> (ø)
src/index.js	100.00% <100.00%> (ø)
src/locale/en.js	100.00% <100.00%> (ø)
src/locale/fr.js	100.00% <100.00%> (ø)
src/locale/nl.js	100.00% <100.00%> (ø)
src/locale/zh-tw.js	100.00% <100.00%> (ø)
src/plugin/advancedFormat/index.js	100.00% <100.00%> (ø)
src/plugin/bigIntSupport/index.js	100.00% <100.00%> (ø)
src/plugin/customParseFormat/index.js	100.00% <100.00%> (ø)
src/plugin/customParseFormat/utils.js	100.00% <100.00%> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

thanks, please allow me some time to go through this cool PR

You've got all the time in the world; if my documentation of the changes needs improvement, feel free to ask.

@BePo65 any chance you could update this PR soon? Or would you be fine if I tried to update it in a new PR?

@BePo65 any chance you could update this PR soon? Or would you be fine if I tried to update it in a new PR?

I added 2 tests for your issue #1734 and found out that I missed a comment from @iamkun - replied to this comment too.

So now I hope we have everything in place so that the review of this pr can be completed. Let's hope the best 😄

Is any help needed with this PR?

@le0pard you can review this PR, but mostly we're waiting on a review by @iamkun