EvanHahn / HumanizeDuration.js

361000 becomes "6 minutes, 1 second"
https://evanhahn.github.io/HumanizeDuration.js/
The Unlicense
1.65k stars 175 forks source link

Incorrect values for even days, months, years #152

Closed anthonykulis closed 5 years ago

anthonykulis commented 5 years ago

To replicate:

v3.18.0

You can do this simply on your demo page

Lets assume 35 days: 1000 60 60 24 35 = 3024000000ms

Result should be either 1 month and 4 days (assuming 31 day month) or 1 month 5 days. Get the screen shot value.

This issue follows for even months and years as well.

Screen Shot 2019-04-30 at 12 41 27 PM

EvanHahn commented 5 years ago

Thanks for reporting. I'll plan to take a look at this soon.

EvanHahn commented 5 years ago

This is probably because of how we define months. Months are defined internally as 2,629,800,000 milliseconds, which is 30.4375 days. That's because that's the mean length of a month. (Note that it's a little more complicated than 365 ÷ 12 because of leap years—we calculate the average over four years.)

You can fix this by specifying your own unitMeasures in a new humanizer, like so:

const thirtyDays = 30 * 24 * 60 * 60 * 1000

const myHumanizer = humanizeDuration.humanizer({
  unitMeasures: {
    y: 31557600000,
    mo: thirtyDays,
    w: 604800000,
    d: 86400000,
    h: 3600000,
    m: 60000,
    s: 1000,
    ms: 1
  }
})

console.log(myHumanizer(3024000000))
// => 1 month, 5 days

Closing because I think I've answered your question, but let me know if that's wrong and I'll reopen!

alamothe commented 5 years ago

Given that this lib is based on milliseconds, it is completely useless.

EvanHahn commented 5 years ago

@alamothe Sorry you feel that way! I chose to have this library based on milliseconds because that seems to be the "base time unit" in JavaScript (for example, that's what Date.now() returns).

alamothe commented 5 years ago

Sorry for the harsh comment, it should be fine for displaying approximate duration. Unfortunately for my case it has be to a precise duration between two datetimes in a certain timezone, which needs to account DST. This seems to be hard to find in JavaScript world (even moment don't account for DST)

anthonykulis commented 5 years ago

The only problem with this is months will never be 30.4375 days. A month is enumerated a either 28 days, 29 days, 30 days, or 31 days. My suggestion is to be opinionated and just say a month is 30 days.

EvanHahn commented 5 years ago

@alamothe This library just turns milliseconds into humanized durations, and doesn't know anything about timezones, so I expect yours to be a separate issue. (But maybe that's wrong!)

@anthonykulis That's reasonable. Unfortunately, this library is in maintenance mode and that would be a breaking change. The good news is that you can follow these steps to set it to 30 days yourself.

IDisposable commented 5 years ago

https://github.com/EvanHahn/HumanizeDuration.js/issues/152#issuecomment-494847931

Yes, but this library doesn't address a month (e.g. not a specific point-in-time in a calendar). Rather, this library humanizes the concept of the elapsed time as expressed in milliseconds to a Gregorian-based elapsed time expression. It doesn't talk about specific dates, so it cannot include leap days and lengths of specific months. It doesn't talk about leap seconds. It just talks about "how many X is Y milliseconds" in a reasonable way.