Inconsistency with +moment.duration() and moment.duration().asMilliseconds()

[rw103]

Describe the bug
+moment.duration({ months: 2 }) returns 5184000000 (60 days) but moment.duration({ months: 2 }).asMilliseconds() returns 5270400000 (61 days)

To Reproduce
Run the following code in Node.

const moment = require('moment');

console.log(+moment.duration({ months: 2 }));
console.log(moment.duration({ months: 2 }).asMilliseconds());

Expected behavior
I expect the following output (60 days in ms for both):

5184000000
5184000000

Instead, I get this output:

5184000000
5270400000

Desktop (please complete the following information):

• OS: Mac 10.14.6
• Node 10.15.3

Moment-specific environment

• The time zone setting of the machine the code is running on
• The time and date at which the code was run
• Other libraries in use (TypeScript, Immutable.js, etc)

Please run the following code in your environment and include the output:

console.log((new Date()).toString())
console.log((new Date()).toLocaleString())
console.log((new Date()).getTimezoneOffset())
console.log(moment.version)

Output:

Wed Jun 17 2020 16:02:19 GMT-0700 (Pacific Daylight Time)
6/17/2020, 4:02:19 PM
420
2.26.0

[Alanscut]

I checked the source code, and I think the calculation based on 61 days is more accurate, what do you think @rw103 @marwahaha? I am happy to submit a pull request 😄 .

Source Code

duration/as.js

This valueOf() is calculated using 30 days.

// TODO: Use this.as('ms')?
export function valueOf() {
    if (!this.isValid()) {
        return NaN;
    }
    return (
        this._milliseconds +
        this._days * 864e5 +
        (this._months % 12) * 2592e6 +
        toInt(this._months / 12) * 31536e6
    );
}

duration/as.js and duration/bubble.js

This as() is calculated using the 400-year average

function as(units) {
        // ...
        // handle milliseconds separately because of floating point math errors (issue #1867)
        days = this._days + Math.round(monthsToDays(this._months));
       // ...
}

function monthsToDays(months) {
    // the reverse of daysToMonths
    return (months * 146097) / 4800;
}

[MinusFour]

I think this could be a breaking change out there in the wild. It does kind of make sense for both functions to return the same amount of milliseconds though.

[rw103]

Yeah, I originally said 60 days makes more sense for expected behavior because I didn't know the rationale for 61 days. But it's true that not every month is 30 days, so using an average month duration makes sense. The more important thing is to make both functions consistent.

[amigian74]

I have a similar problem.

let maxTimeRange = moment.duration(1, 'years');
let end = moment('2020-12-31 23:59:59');
let start = moment('2020-01-01 00:00:00');
let duration = moment.duration(end.diff(start));

Unfortunately maxTimeRange is less than duration

[DavidHenri008]

I have a similar issue where I convert 90 months into milliseconds and then back into months. This offset is caused by the rounding:

// handle milliseconds separately because of floating point math errors (issue #1867)
days = this._days + Math.round(monthsToDays(this._months));

I do not understand why the 365.25/12 is not simply used as a conversion factor.