Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duration is wrong on 1.11.10 #2464

Open
adrien-may opened this issue Sep 26, 2023 · 6 comments
Open

Duration is wrong on 1.11.10 #2464

adrien-may opened this issue Sep 26, 2023 · 6 comments

Comments

@adrien-may
Copy link

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
@bombillazo
Copy link

Any update on this error?

@pvds
Copy link

pvds commented Nov 24, 2023

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

@sanjaynishad
Copy link
Contributor

@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

@yanny7
Copy link

yanny7 commented Dec 14, 2023

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

@AndrewGnagy
Copy link

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?

@vad99lord
Copy link

vad99lord commented Apr 12, 2024

looks like #2583 addresses this issue with missing weeks handling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants