Skip to content

Commit

Permalink
[REF] dates: introduce DateTime to wrap Date object
Browse files Browse the repository at this point in the history
This commit prepares the ground for a fix (next commit).

The issue
---------

Steps to reproduce:

- change your machine timezone to Jakarta
- enter "3/8/2023" in a cell
=> it becomes "3/7/2023"

Other steps to reproduce:

- change your machine timezone to Jakarta
- npm run test
=> some tests are failing.

Spreadsheet date times are naive date times, they do not carry any timezone
information. A date(time) in spreadsheet is just a number with a format
after all.

We are currently using javascript native Date objects everywhere in the code to
represent spreadsheet dates.
And we are mixing UTC (new Date(timestamp)) and localized Dates
(new Date(year, month, day, ...)) at some places. This is a mistake.

Because of an (un)lucky combination of mistakes, it mostly works by chance for
most timezones.
But going back to the faulty Jakarta case: one can notice that
INITIAL_1900_DAY is UTC+0707 while INITIAL_JS_DAY is UTC+0700 (7 minutes
offset). The timezone offset changed over time.

Some of the mistakes where we mix up things:

- INITIAL_1900_DAY is localized, but INITIAL_JS_DAY is UTC.
- numberToJsDate: we create the date from the value as if it was UTC,
  but then set the time as if it was localized.

This commit
-----------

This commit prepares the ground for the real fix (next commit). The idea to
fix the issue is to avoid mixing localized and UTC Dates. To represent naive
date times, we will always use UTC Date objects.

we introduce a wrapper around the Date native object for two reasons:
- the business code should not know or care about timezones/UTC
- manipulating Date is error prone. It's easy to mess up local vs UTC
  (as this bug fix has shown)

With this [REF] commit, the behavior does not change. We still mix localised
(`new DateTime(...)`) and UTC (`DateTime.fromTimestamp(...)`).
The change comes with the next commit.

For the future, we could also improve the `DateTime` API, compared to the poor
`Date` API. For stable version however, we keep the changes to the minimum and
keep the Date API

Task: 3666703
Part-of: #3396
  • Loading branch information
LucasLefevre committed Jan 8, 2024
1 parent 668e65b commit fea81ee
Show file tree
Hide file tree
Showing 7 changed files with 266 additions and 197 deletions.
2 changes: 1 addition & 1 deletion src/functions/helper_financial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export function assertSettlementLessThanOneYearBeforeMaturity(
const startDate = toJsDate(settlement);
const endDate = toJsDate(maturity);

const startDatePlusOneYear = new Date(startDate);
const startDatePlusOneYear = toJsDate(settlement);
startDatePlusOneYear.setFullYear(startDate.getFullYear() + 1);

assert(
Expand Down
4 changes: 2 additions & 2 deletions src/functions/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// HELPERS
import { memoize } from "../helpers";
import { numberToJsDate, parseDateTime } from "../helpers/dates";
import { DateTime, numberToJsDate, parseDateTime } from "../helpers/dates";
import { isNumber, parseNumber } from "../helpers/numbers";
import { _lt } from "../translation";
import { ArgValue, CellValue, MatrixArgValue, PrimitiveArgValue } from "../types";
Expand Down Expand Up @@ -123,7 +123,7 @@ function strictToBoolean(value: string | number | boolean | null | undefined): b
return toBoolean(value);
}

export function toJsDate(value: string | number | boolean | null | undefined): Date {
export function toJsDate(value: string | number | boolean | null | undefined): DateTime {
return numberToJsDate(toNumber(value));
}

Expand Down
48 changes: 24 additions & 24 deletions src/functions/module_date.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
addMonthsToDate,
DateTime,
getYearFrac,
INITIAL_1900_DAY,
jsDateToRoundNumber,
Expand Down Expand Up @@ -46,7 +47,7 @@ export const DATE: AddFunctionDescription = {
_year += 1900;
}

const jsDate = new Date(_year, _month - 1, _day);
const jsDate = new DateTime(_year, _month - 1, _day);
const result = jsDateToRoundNumber(jsDate);

assert(
Expand Down Expand Up @@ -187,7 +188,7 @@ export const EOMONTH: AddFunctionDescription = {

const yStart = _startDate.getFullYear();
const mStart = _startDate.getMonth();
const jsDate = new Date(yStart, mStart + _months + 1, 0);
const jsDate = new DateTime(yStart, mStart + _months + 1, 0);
return jsDateToRoundNumber(jsDate);
},
isExported: true,
Expand Down Expand Up @@ -234,19 +235,19 @@ export const ISOWEEKNUM: AddFunctionDescription = {
// Thursday of the year.

let firstThursday = 1;
while (new Date(y, 0, firstThursday).getDay() !== 4) {
while (new DateTime(y, 0, firstThursday).getDay() !== 4) {
firstThursday += 1;
}
const firstDayOfFirstWeek = new Date(y, 0, firstThursday - 3);
const firstDayOfFirstWeek = new DateTime(y, 0, firstThursday - 3);

// The last week of the year is the week that contains the last Thursday of
// the year.

let lastThursday = 31;
while (new Date(y, 11, lastThursday).getDay() !== 4) {
while (new DateTime(y, 11, lastThursday).getDay() !== 4) {
lastThursday -= 1;
}
const lastDayOfLastWeek = new Date(y, 11, lastThursday + 3);
const lastDayOfLastWeek = new DateTime(y, 11, lastThursday + 3);

// B - If our date > lastDayOfLastWeek then it's in the weeks of the year after
// If our date < firstDayOfFirstWeek then it's in the weeks of the year before
Expand All @@ -266,25 +267,25 @@ export const ISOWEEKNUM: AddFunctionDescription = {
// the first day of this year and the date. The difference in days divided by
// 7 gives us the week number

let firstDay: Date;
let firstDay: DateTime;
switch (offsetYear) {
case 0:
firstDay = firstDayOfFirstWeek;
break;
case 1:
// firstDay is the 1st day of the 1st week of the year after
// firstDay = lastDayOfLastWeek + 1 Day
firstDay = new Date(y, 11, lastThursday + 3 + 1);
firstDay = new DateTime(y, 11, lastThursday + 3 + 1);
break;
case -1:
// firstDay is the 1st day of the 1st week of the previous year.
// The first week of the previous year is the week that contains the
// first Thursday of the previous year.
let firstThursdayPreviousYear = 1;
while (new Date(y - 1, 0, firstThursdayPreviousYear).getDay() !== 4) {
while (new DateTime(y - 1, 0, firstThursdayPreviousYear).getDay() !== 4) {
firstThursdayPreviousYear += 1;
}
firstDay = new Date(y - 1, 0, firstThursdayPreviousYear - 3);
firstDay = new DateTime(y - 1, 0, firstThursdayPreviousYear - 3);
break;
}

Expand Down Expand Up @@ -466,8 +467,8 @@ export const NETWORKDAYS_INTL: AddFunctionDescription = {
}

const invertDate = _startDate.getTime() > _endDate.getTime();
const stopDate = new Date((invertDate ? _startDate : _endDate).getTime());
let stepDate = new Date((invertDate ? _endDate : _startDate).getTime());
const stopDate = DateTime.fromTimestamp((invertDate ? _startDate : _endDate).getTime());
let stepDate = DateTime.fromTimestamp((invertDate ? _endDate : _startDate).getTime());
const timeStopDate = stopDate.getTime();
let timeStepDate = stepDate.getTime();

Expand Down Expand Up @@ -496,8 +497,7 @@ export const NOW: AddFunctionDescription = {
returns: ["DATE"],
computeFormat: () => "m/d/yyyy hh:mm:ss",
compute: function (): number {
let today = new Date();
today.setMilliseconds(0);
let today = DateTime.now();
const delta = today.getTime() - INITIAL_1900_DAY.getTime();
const time = today.getHours() / 24 + today.getMinutes() / 1440 + today.getSeconds() / 86400;
return Math.floor(delta / MS_PER_DAY) + time;
Expand Down Expand Up @@ -589,8 +589,8 @@ export const TODAY: AddFunctionDescription = {
returns: ["DATE"],
computeFormat: () => "m/d/yyyy",
compute: function (): number {
const today = new Date();
const jsDate = new Date(today.getFullYear(), today.getMonth(), today.getDate());
const today = DateTime.now();
const jsDate = new DateTime(today.getFullYear(), today.getMonth(), today.getDate());
return jsDateToRoundNumber(jsDate);
},
isExported: true,
Expand Down Expand Up @@ -663,11 +663,11 @@ export const WEEKNUM: AddFunctionDescription = {
const y = _date.getFullYear();

let dayStart = 1;
let startDayOfFirstWeek = new Date(y, 0, dayStart);
let startDayOfFirstWeek = new DateTime(y, 0, dayStart);

while (startDayOfFirstWeek.getDay() !== startDayOfWeek) {
dayStart += 1;
startDayOfFirstWeek = new Date(y, 0, dayStart);
startDayOfFirstWeek = new DateTime(y, 0, dayStart);
}

const dif = (_date.getTime() - startDayOfFirstWeek.getTime()) / MS_PER_DAY;
Expand Down Expand Up @@ -750,7 +750,7 @@ export const WORKDAY_INTL: AddFunctionDescription = {
});
}

let stepDate = new Date(_startDate.getTime());
let stepDate = DateTime.fromTimestamp(_startDate.getTime());
let timeStepDate = stepDate.getTime();

const unitDay = Math.sign(_numDays);
Expand Down Expand Up @@ -847,7 +847,7 @@ export const MONTH_START: AddFunctionDescription = {
const _startDate = toJsDate(date);
const yStart = _startDate.getFullYear();
const mStart = _startDate.getMonth();
const jsDate = new Date(yStart, mStart, 1);
const jsDate = new DateTime(yStart, mStart, 1);
return jsDateToRoundNumber(jsDate);
},
};
Expand Down Expand Up @@ -894,7 +894,7 @@ export const QUARTER_START: AddFunctionDescription = {
compute: function (date: PrimitiveArgValue): number {
const quarter = QUARTER.compute(date) as number;
const year = YEAR.compute(date) as number;
const jsDate = new Date(year, (quarter - 1) * 3, 1);
const jsDate = new DateTime(year, (quarter - 1) * 3, 1);
return jsDateToRoundNumber(jsDate);
},
};
Expand All @@ -912,7 +912,7 @@ export const QUARTER_END: AddFunctionDescription = {
compute: function (date: PrimitiveArgValue): number {
const quarter = QUARTER.compute(date) as number;
const year = YEAR.compute(date) as number;
const jsDate = new Date(year, quarter * 3, 0);
const jsDate = new DateTime(year, quarter * 3, 0);
return jsDateToRoundNumber(jsDate);
},
};
Expand All @@ -929,7 +929,7 @@ export const YEAR_START: AddFunctionDescription = {
computeFormat: () => "m/d/yyyy",
compute: function (date: PrimitiveArgValue): number {
const year = YEAR.compute(date) as number;
const jsDate = new Date(year, 0, 1);
const jsDate = new DateTime(year, 0, 1);
return jsDateToRoundNumber(jsDate);
},
};
Expand All @@ -946,7 +946,7 @@ export const YEAR_END: AddFunctionDescription = {
computeFormat: () => "m/d/yyyy",
compute: function (date: PrimitiveArgValue): number {
const year = YEAR.compute(date) as number;
const jsDate = new Date(year + 1, 0, 0);
const jsDate = new DateTime(year + 1, 0, 0);
return jsDateToRoundNumber(jsDate);
},
};

0 comments on commit fea81ee

Please sign in to comment.