moment / luxon

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

DateTime.minus() returns wrong value #1388

Closed cbouwense closed 1 year ago

cbouwense commented 1 year ago

Describe the bug Subtracting n days, where n > 64, seems to only subtract n-1 days + 23 hours.

To Reproduce

// Example of bug.
// This evaluates to 1331387600, one hour after the desired time (incorrect!).
DateTime
  .fromSeconds(1337000000)
  .minus({ days: 65 })
  .toSeconds();

// Example of good behavior using `hours`.
// This evaluates to 1331384000, exactly the time desired.
DateTime
  .fromSeconds(1337000000)
  .minus({ hours: 65 * 24 }) // <--- Manually using hours
  .toSeconds();

// Example of good behavior with days <= 64.
// This evaluates to 1331470400, exactly the time desired.
DateTime
  .fromSeconds(1337000000)
  .minus({ days: 65 * 24 })
  .toSeconds();

Actual vs Expected behavior Actual Given days greater than 64, the subtraction calculation is off by one hour. Expected Given any value for days, the calculation should be exactly correct.

Desktop (please complete the following information):

Additional context None

diesieben07 commented 1 year ago

You are observing the effects of daylight saving time. Not every day is 24 hours long. Subtracting 65 days subtracts calendar days, i.e. it keeps the wall time the same. See the difference between "calendar math" and "time math" in the documentation.

Example to make the effect more clear:

let dt = DateTime.fromSeconds(1337000000, {zone: 'America/New_York'});
console.log(dt.toISO()); 
// "2012-05-14T08:53:20.000-04:00" - our starting point

console.log(dt.minus({days: 65}).toISO()); 
// "2012-03-10T08:53:20.000-05:00" - observe the identical wall time but different UTC offset

console.log(dt.minus({hours: 65 * 24}).toISO());
// "2012-03-10T07:53:20.000-05:00" // exactly 65 * 24 hours were subtracted, this reduced the wall time by one because a DST border was crossed

Note: The number 65 is a fluke here. It appears, because that is the number of days needed given your starting point to cross a DST border.

cbouwense commented 1 year ago

That is super interesting, thank you for clearing that up!