dayjs icon indicating copy to clipboard operation
dayjs copied to clipboard

Duration is wrong on 1.11.10

Open adrien-may opened this issue 1 year ago • 6 comments

Describe the bug The duration is wrong.

Let's take this example

// Init
const dayjs = require('dayjs')
const duration = require( 'dayjs/plugin/duration');

dayjs.extend(duration);

// two dates
const first = dayjs('2020-01-01T12:00:00.000Z')
const start = dayjs('2020-01-01T12:00:00.000Z').subtract(10, 'weeks')

// compute duration
const milliseconds = first.diff(start, 'milliseconds')
dayjs.duration(milliseconds).toISOString()

With dayjs 1.11.09, we get 'P2M10DT1H' With dayjs 1.11.10 we get 'P2M9DT5H' which I don't even understand

Expected behavior

Previous behavior is right

Information

  • Day.js Version 1.11.09 vs 1.11.10

adrien-may avatar Sep 26 '23 09:09 adrien-may

Any update on this error?

bombillazo avatar Oct 09 '23 16:10 bombillazo

@iamkun @sanjaynishad can this behaviour be caused by #2362?

pvds avatar Nov 24 '23 11:11 pvds

@pvds I don't think, because duration was broken in 1.11.9 as well, and maybe someone pushed the fixes and that's causing the issue

#2479 when I tried this with 1.11.9

const today = dayjs()
const twoWeeksFromNow = dayjs().add(dayjs.duration({ weeks: 2 }))

console.log({ today, twoWeeksFromNow })

I got

{
  today: M {
    '$L': 'en',
    '$d': 2023-11-30T14:03:05.248Z,
    '$x': {},
    '$y': 2023,
    '$M': 10,
    '$D': 30,
    '$W': 4,
    '$H': 19,
    '$m': 33,
    '$s': 5,
    '$ms': 248
  },
  twoWeeksFromNow: M {
    '$L': 'en',
    '$d': Invalid Date,
    '$x': {},
    '$y': NaN,
    '$M': NaN,
    '$D': NaN,
    '$W': NaN,
    '$H': NaN,
    '$m': NaN,
    '$s': NaN,
    '$ms': NaN
  }
}

I'll try to investigate more on this

sanjaynishad avatar Nov 30 '23 14:11 sanjaynishad

Issue is caused by wrong parsing from milliseconds

    _proto.parseFromMilliseconds = function parseFromMilliseconds() {
        var $ms = this.$ms;
        this.$d.years = roundNumber($ms / MILLISECONDS_A_YEAR);
        $ms %= MILLISECONDS_A_YEAR;
        this.$d.months = roundNumber($ms / MILLISECONDS_A_MONTH);
        $ms %= MILLISECONDS_A_MONTH;
        this.$d.days = roundNumber($ms / MILLISECONDS_A_DAY);
        ....

because there is used wrong constant for months:

var MILLISECONDS_A_YEAR = MILLISECONDS_A_DAY * 365;
var MILLISECONDS_A_MONTH = MILLISECONDS_A_YEAR / 12;

This means that one month is 30.41666 days long, causing problems for duration bigger than 1 month

yanny7 avatar Dec 14 '23 15:12 yanny7

Got curious and did some research. According to this version of ISO 8601 doc: 2.2.12 month "duration of 28, 29, 30 or 31 calendar days depending on the start and/or the end of the corresponding time interval within the specific calendar month" and later "In certain applications a month is considered as a duration of 30 calendar days"

I believe this favors the original implementation of var MILLISECONDS_A_MONTH = MILLISECONDS_A_DAY * 30?

AndrewGnagy avatar Dec 14 '23 20:12 AndrewGnagy

looks like #2583 addresses this issue with missing weeks handling

vad99lord avatar Apr 12 '24 15:04 vad99lord