iamkun / dayjs

⏰ Day.js 2kB immutable date-time library alternative to Moment.js with the same modern API
https://day.js.org
MIT License
47k stars 2.3k forks source link

Duration format result is not as expected #1521

Closed awenger63 closed 3 years ago

awenger63 commented 3 years ago

Describe the bug When use duration format for an ISO8601, result it's not as expected : myDuration = dayjs.duration('PT3M39.096S') myDuration.format("HH:mm:ss") => "NaN:03:39.096" myDuration.format("mm:ss") => "03:39.096" myDuration.format("ss") => "39.096" myDuration.format("SSS") => "undefined"

Expected behavior myDuration.format("HH:mm:ss") => "00:03:39" myDuration.format("mm:ss") => "03:39" myDuration.format("ss") => "39" myDuration.format("SSS") => "096"

Information

awenger63 commented 3 years ago

Did you take a look at this issue ? Dou you need more test case ?

michaelnguyenm commented 3 years ago

I'm actually having a similar issue with the above problem with regards to NaN and would like to see it fixed if possible.

Upon inspecting the code here: https://github.com/iamkun/dayjs/blob/5a79cc6408e825d9e123629eb44fc19c996d7751/src/plugin/duration/index.js#L83-L100

Which uses the regex: https://github.com/iamkun/dayjs/blob/5a79cc6408e825d9e123629eb44fc19c996d7751/src/plugin/duration/index.js#L13

It appears that there are multiple issues with regards to parsing ISO 8601 duration strings. The first is that some capture groups will not always be present as defined by ISO 8601, resulting in undefined after using String.prototype.match(). These values should be converted as 0 instead of NaN when converted into a Number. This is apparent when using an input value such as PT3H19M where the seconds will be missing, so the seconds will be rendered as NaN. See:

const input = 'PT3H19M';
const regex = /^(-|\+)?P(?:([-+]?[0-9,.]*)Y)?(?:([-+]?[0-9,.]*)M)?(?:([-+]?[0-9,.]*)W)?(?:([-+]?[0-9,.]*)D)?(?:T(?:([-+]?[0-9,.]*)H)?(?:([-+]?[0-9,.]*)M)?(?:([-+]?[0-9,.]*)S)?)?$/;
const found = input.match(regex);
console.log(found);
// > Array ["PT3H19M", undefined, undefined, undefined, undefined, undefined, "3", "19", undefined]

The second issue is that the regex for seconds will capture both integers and decimals. When converted to a Number, the decimal will be kept when the duration object $d is should have separate properties for seconds and milliseconds. In other words, they should be parsed separately.

cypressious commented 3 years ago

I've just hit this problem and I've found that calling duration.add(0, 'seconds') on a broken instance of Duration will return an instance where the NaN components will be replaced with 0s.

iamkun commented 3 years ago

:tada: This issue has been resolved in version 1.10.7 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

awenger63 commented 3 years ago

Hello, @cypressious & @iamkun , thanks a lot for your action, but new issue #1665 is open for complet solve this For help improvement ;-)

adroste commented 2 years ago

@iamkun I'm on 1.10.7 and this is definitely not fixed. See #1665