Skip to content
This repository has been archived by the owner on Jul 22, 2022. It is now read-only.

Fix getExceTS daylight savings bug #333

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

finnshort
Copy link

@finnshort finnshort commented Feb 22, 2021

In getExcelTS, two days are added to the submitted date, for two separate reasons:

  1. To get the excel serial date, the 'epoch' is subtracted from our date. When using the 1900 date system, the epoch is 1900-01-01, or day '1' in serial. The first additional day is added to our date to account for this subtraction. It is accounted for here:
    let thisDt = new Date(date);
    thisDt.setDate(thisDt.getDate() + 1);
  1. There is a known bug where Excel calculates 1900 as a leap year wen it wasn't, so the extra fictitious day Feb 29 1900 is accounted for in this block:
    // Handle legacy leap year offset as described in  §18.17.4.1
    const legacyLeapDate = new Date('1900-02-28T23:59:59.999Z');
    if (thisDt - legacyLeapDate > 0) {
        thisDt = new Date(thisDt.getTime() + 24 * 60 * 60 * 1000);
    }

The days are then subtracted back out here:

    let epoch = new Date('1900-01-01T00:00:00.0000Z');
    let diff2 = thisDt.getTime() - epoch.getTime();

However, a bug arises (as described in #324) when the two days are added over a daylight savings change. For example, when the UTC timestamp '2020-03-06T15:38:00Z' is passed to getExcelTS, it is received as Fri Mar 06 2020 07:38:00 GMT-0800 (Pacific Standard Time). The first addition day becomes Sat Mar 07 2020 07:38:00 GMT-0800 (Pacific Standard Time), then the second additional day crosses a daylight savings change and becomes Sun Mar 08 2020 07:38:00 GMT-0700 (Pacific Daylight Time). This is a change of 23 hours rather than 24. Although only 47 hours have been added, 2 full days are still subtracted, resulting in the output time being 1 hour early/behind. When daylight savings 'falls back' in the fall 49 hours are added and the times are 1 hour ahead.

Note that this occurs event if getExcelTS is passed a UTC date because javascript date object methods work in the local time zone of the host system.

Adding 24 hours instead of 1 day solves this bug without breaking any of the existing tests.

Thanks to @n-a-t-e for the gist identifying the bug and @mactyr for writing the solution! #324

Resources:
ECMA-376, Second Edition, Part 1 - Fundamentals And Markup Language Reference Section 18.17.4 Dates and Times
https://www.myonlinetraininghub.com/excel-date-and-time
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date

@arthurblake-AngelOak
Copy link

Unfortunately It looks like this excellent project might be abandoned by the author...
I hope @natergj is alive and well and on to better things.
I think our best bet going forward is this fork: https://www.npmjs.com/package/@advisr/excel4node
I asked @jrohland nicely to merge this PR and it's done!
See advisr-io#4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants