Fix 2.11->2.12 DST issues with differenceInDays (fix #1750)

[justingrant]

As documented in #1750, date-fns 2.12 (specifically #1630) contained a breaking change that made differenceInDays stop matching addDays across a DST boundary. This PR replaces #1630 with an implementation that fixes #533 (like #1630 did) without causing #1750 (like #1630 also did).

This PR:

• Fixes differenceInDays no longer matches addDays (breaking change in 2.12?) #1750 by restoring the 2.11 implementation of differenceInDays (so should code-review against 2.11 implementation) but modifying it to fix Incorrect result across DST in differenceInDays #533 without triggering differenceInDays no longer matches addDays (breaking change in 2.12?) #1750. It does this by replacing the use in differenceInDays of compareAsc with a new local function (compareLocalAsc) that compares using local time not UTC timestamps. This change prevents errors when one of the times is on a DST boundary or contains an ambiguous local time due to DST.
• Adds new DST tests for differenceInDays to validate the work above. These tests will work in any timezone, thanks to new shared test code that detects DST start in the local timezone. All DST tests in this PR are relative to the DST start/end dates detected for the current timezone. These tests will be skipped in non-DST time zones.
• Validates that all tests run OK in the America/Los_Angeles, America/Sao_Paulo, Australia/Sydney, and Europe/London timezones. This was done by temporarily adding a TZ env to the yarn test script command and then manually running tests under each TZ.
• Adds new JSDoc content and samples for differenceInDays to document DST behavior and to explain how to get non-DST behavior (full 24-hour periods only) using Math.floor(differenceInHours(dateLeft, dateRight)/24)|0.
• Adds analogous JSDoc content to differenceInWeeks which is implemented using differenceInDays and shares its DST behavior.
• Updates the changelog with a new empty release section as requested in CONTRIBUTING.md,

Fix Details

#533, like most JS bugs related to DST, happened because the pre-2.12 implementation of differenceInDays mixed UTC and local date calculations: CompareAsc uses UTC comparisons while Date.setDate (used in differenceInDays) uses local timezone math. The result: on or around DST boundaries CompareAsc returned results which suggested that the difference was not a full day when it really was, or vice versa. This led to off-by-one errors in results like #533.

The fix was to restore the 2.11 implementation of differenceInDays (effectively reverting #1630) and simply replace usage of compareAsc with a new local function compareLocalAsc that uses local date math instead.

If you really want to get into the weeds to understand how setDate works and why #533 happened, start here:

• https://tc39.es/ecma262/#sec-date.prototype.setdate
• https://tc39.es/ecma262/#sec-local-time-zone-adjustment

Discussion

"Full Days": 24-hour periods vs. matching local times

Some contributors in #533 suggested that date diffs should be measured in 24-hour periods only and should ignore DST. To see why this is problematic, imagine a user measures a 7-day period from (local) noon to (local) noon one week later. If we only diff 24-hour periods, then that period would measure 7 days most of the year, but would return 6 days if DST started in the middle. This intermittent variation is a certain cause of hard-to-diagnose bugs, and is also different from how date-fns has behaved for a long time. That's why this PR goes back to the 2.11 behavior of taking DST into account. BTW, if a developer wants to ignore DST and just count 24-hour periods, this PR documents one way (Math.floor(differenceInHours(dateLeft, dateRight)/24)|0) in user-visible JSDoc comments.

API Consistency

I admit that I couldn't fully understand the consistency discussion in #533. To me it seems like the pre-2.12 behavior was more consistent, because in 2.11 differenceInDays was consistent with addDays, and differenceInDays was also consistent with differenceInMonths. And with JS Date APIs like Date.setDate.

Guidelines

In case it's helpful, here are three general rules of thumb that I use for safe date math:

• For adding/subtracting/comparing hours (and smaller) units, convert to UTC and do all math in UTC.
• For adding/subtracting/comparing days (and larger) units, only use local timezone math, not UTC (unless UTC is the local timezone!) because users expect that day and larger APIs return the same results whether or not a DST transition happened. Here's a discussion about the same topic in moment.js: Adding across DST results in a difference of an hour moment/moment#78 (comment)
• Know which APIs use local vs. UTC math, and don't mix them unless you really, really know what you're doing. When you mix UTC APIs like Date.getTime with local APIs like Date.setDate, it's easy to end up with off-by-one-hour bugs like Incorrect result across DST in differenceInDays #533 or Starting with v2.0.0-beta.1 date-fns doesn't accept strings as arguments. Please use parseISO to parse strings #1083 1510.

Future Investigation Needed

TODO: I suspect that differenceInMonths, differenceInISOWeekYears, and differenceInYears are vulnerable to the same bug but I didn't have the time yet to investigate but I wanted to get this breaking-change fix out ASAP.

[justingrant]

My new tests failed in CI for the Atlantic/Azores timezone. Interestingly, the tests failed for differenceInCalendarDays whose implementation didn't change in this PR, so either my tests are bad or there's a DST problem with differenceInCalendarDays. I can repro this failure locally and I'll investigate and push a fix, probably tomorrow.

Luckily the DST config for Atlantic/Azores is unusual: their standard time is UTC-1 and their DST is UTC, so the problem case seems pretty obvious: transitions to or from DST across or on local date boundaries and/or UTC date boundaries.

Kudos to the date-fns team for a great CI setup to catch weird bugs like this!

[kossnocorp]

Thank you so much for detailed research, your help is invaluable 👏

[justingrant]

OK, fixed the problem with Atlantic/Azores. The culprit was my test code to find DST boundaries, which broke if those boundaries were at midnight. Fixed now. Also I rebased to the current master and put my changelog edits on top.

Should be ready to merge.

[kossnocorp]

🙏 Such great work, thank you!

[kossnocorp]

I've shipped the change with date-fns@2.13.0, thank you again!