natergj / excel4node

Node module to allow for easy Excel file creation
MIT License
1.38k stars 215 forks source link

Fix getExceTS daylight savings bug #333

Open finnshort opened 3 years ago

finnshort commented 3 years ago

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);
  2. 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! https://github.com/natergj/excel4node/issues/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 commented 2 years ago

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 https://github.com/advisr-io/excel4node/pull/4