moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.12k stars 728 forks source link

Unexpected / unintuitive behaviour of Duration.minus() #1515

Closed ipg0 closed 9 months ago

ipg0 commented 9 months ago

Describe the bug Subtracting seconds from a Duration defined in days results in unchanged number of days and negative seconds, which might not be expected behaviour.

Also, couldn't find any documentation on whether this might be expected behaviour, so I would assume it is not.

To Reproduce Here's the issue reproduced in Node REPL v18.18.0

Welcome to Node.js v18.18.0.
Type ".help" for more information.
> const luxon = require('luxon')
undefined
> let d = luxon.Duration.fromObject({days: 7})
undefined
> let d1 = d.minus({seconds: 1})
undefined
> d1.toObject()
{ days: 7, seconds: -1 }

Actual vs Expected behavior Subtraction of seconds from a Duration is, presumably, expected to result in a Duration object containing a valid representation of a duration of time. However, the result above (7 days and -1 seconds) is clearly not a valid representation of it.

Desktop (please complete the following information):

icambron commented 9 months ago

It is a valid representation. In fact, it's the only one that is completely clean: days are not necessarily all the same length, so to do basically anything else, Luxon would have to do some converting in not-completely-always-right ways. A rule we try to follow with Luxon is to not do any conversions unless you ask for it.

Which you can do:

d1.normalize().toObject() //=> { days: 6, seconds: 86399 } 
d1.shiftTo("days").toOject() // =>  { days: 6.999988425925926 } 
d1.shiftTo("days", "hours", "minutes", "seconds").toObject()   //=> { days: 6, hours: 23, minutes: 59, seconds: 59 }