From def2eac1eb90efb933f1db20d1b7039f1c668ff2 Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Fri, 19 Jan 2024 17:01:38 -0700 Subject: [PATCH 1/8] Add benchmark for DateTime.local with zone --- benchmarks/datetime.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/benchmarks/datetime.js b/benchmarks/datetime.js index 39d6bf07c..5def68fae 100644 --- a/benchmarks/datetime.js +++ b/benchmarks/datetime.js @@ -9,7 +9,7 @@ function runDateTimeSuite() { const dt = DateTime.now(); suite - .add("DateTime.local", () => { + .add("DateTime.now", () => { DateTime.now(); }) .add("DateTime.fromObject with locale", () => { @@ -18,6 +18,9 @@ function runDateTimeSuite() { .add("DateTime.local with numbers", () => { DateTime.local(2017, 5, 15); }) + .add("DateTime.local with numbers and zone", () => { + DateTime.local(2017, 5, 15, 11, 7, 35, { zone: "America/New_York" }); + }) .add("DateTime.fromISO", () => { DateTime.fromISO("1982-05-25T09:10:11.445Z"); }) From 6072eba65bea0f72ee21df3ff4e02ef1d1286aed Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Wed, 6 Dec 2023 09:50:42 -0500 Subject: [PATCH 2/8] Validate time zone in quickDT prior to guessing offset We validate the zone in the constructor anyway, but the tests missed a branch where we attempt to use an invalid zone to compute an offset, which will result in an exception. Zone#isValid is calculated at creation time (and checked in DateTime constructor already) so this will not degrade performance in the happy case. --- src/datetime.js | 8 ++++++-- test/datetime/invalid.test.js | 20 +++++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/datetime.js b/src/datetime.js index 9472cb531..85533dda0 100644 --- a/src/datetime.js +++ b/src/datetime.js @@ -373,8 +373,12 @@ function normalizeUnitWithLocalWeeks(unit) { // but doesn't do any validation, makes a bunch of assumptions about what units // are present, and so on. function quickDT(obj, opts) { - const zone = normalizeZone(opts.zone, Settings.defaultZone), - loc = Locale.fromObject(opts), + const zone = normalizeZone(opts.zone, Settings.defaultZone); + if (!zone.isValid) { + return DateTime.invalid(unsupportedZone(zone)); + } + + const loc = Locale.fromObject(opts), tsNow = Settings.now(); let ts, o; diff --git a/test/datetime/invalid.test.js b/test/datetime/invalid.test.js index d13fb8643..2fed5bbdb 100644 --- a/test/datetime/invalid.test.js +++ b/test/datetime/invalid.test.js @@ -5,7 +5,11 @@ import { DateTime, Settings } from "../../src/luxon"; const organic1 = DateTime.utc(2014, 13, 33), // not an actual Wednesday organic2 = DateTime.fromObject({ weekday: 3, year: 1982, month: 5, day: 25 }, { zone: "UTC" }), - organic3 = DateTime.fromObject({ year: 1982, month: 5, day: 25, hour: 27 }); + organic3 = DateTime.fromObject({ year: 1982, month: 5, day: 25, hour: 27 }), + organic4 = DateTime.fromObject( + { year: 1982, month: 5, day: 25, hour: 2 }, + { zone: "America/Lasers" } + ); test("Explicitly invalid dates are invalid", () => { const dt = DateTime.invalid("just because", "seriously, just because"); @@ -22,14 +26,28 @@ test("Invalid creations are invalid", () => { test("invalid zones result in invalid dates", () => { expect(DateTime.now().setZone("America/Lasers").isValid).toBe(false); + expect(DateTime.now().setZone("America/Lasers").invalidReason).toBe("unsupported zone"); + expect(DateTime.local({ zone: "America/Lasers" }).isValid).toBe(false); + expect(DateTime.local({ zone: "America/Lasers" }).invalidReason).toBe("unsupported zone"); + + expect(DateTime.local(1982, { zone: "America/Lasers" }).isValid).toBe(false); + expect(DateTime.local(1982, { zone: "America/Lasers" }).invalidReason).toBe("unsupported zone"); + expect(DateTime.fromJSDate(new Date(), { zone: "America/Lasers" }).isValid).toBe(false); + expect(DateTime.fromJSDate(new Date(), { zone: "America/Lasers" }).invalidReason).toBe( + "unsupported zone" + ); + + expect(DateTime.fromMillis(0, { zone: "America/Lasers" }).isValid).toBe(false); + expect(DateTime.fromMillis(0, { zone: "America/Lasers" }).invalidReason).toBe("unsupported zone"); }); test("Invalid DateTimes tell you why", () => { expect(organic1.invalidReason).toBe("unit out of range"); expect(organic2.invalidReason).toBe("mismatched weekday"); expect(organic3.invalidReason).toBe("unit out of range"); + expect(organic4.invalidReason).toBe("unsupported zone"); }); test("Invalid DateTimes can provide an extended explanation", () => { From 5fa43cd5a8b219ff31d3f97d68164dfeec7aae0f Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Mon, 20 Nov 2023 18:31:33 -0500 Subject: [PATCH 3/8] Use computed offset passed in DateTime constructor All calls to the datetime constructor that pass an offset (o), have just computed that offset (quickDT, fromObject) except for clone(). Clone passes an "old" option to determine whether previous offset can be reused. If we have config.o, but config.old is not set, then we can use o without computing it. This saves an expensive call to zone.offset that duplicates work that was done immediately prior. --- src/datetime.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/datetime.js b/src/datetime.js index 9472cb531..06c8b7756 100644 --- a/src/datetime.js +++ b/src/datetime.js @@ -487,7 +487,9 @@ export default class DateTime { if (unchanged) { [c, o] = [config.old.c, config.old.o]; } else { - const ot = zone.offset(this.ts); + // If an offset has been passed and we have not been called from + // clone(), we can trust it and avoid the offset calculation. + const ot = isNumber(config.o) && !config.old ? config.o : zone.offset(this.ts); c = tsToObj(this.ts, ot); invalid = Number.isNaN(c.year) ? new Invalid("invalid input") : null; c = invalid ? null : c; From c5bd5fd539179af4ee4d8685e9c0550851a763eb Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Tue, 21 Nov 2023 14:55:51 -0500 Subject: [PATCH 4/8] Cache ts offset guesses for quickDT Previously we guessed by calling zone.offset(tsNow) on every construction of a date from date parts. This caches that result instead, saving a call to offset --- src/datetime.js | 53 +++++- test/datetime/dst.test.js | 291 ++++++++++++++++++++----------- test/datetime/regexParse.test.js | 23 +++ 3 files changed, 259 insertions(+), 108 deletions(-) diff --git a/src/datetime.js b/src/datetime.js index 9472cb531..4327da6c2 100644 --- a/src/datetime.js +++ b/src/datetime.js @@ -369,13 +369,42 @@ function normalizeUnitWithLocalWeeks(unit) { } } +// cache offsets for zones based on the current timestamp when this function is +// first called. When we are handling a datetime from components like (year, +// month, day, hour) in a time zone, we need a guess about what the timezone +// offset is so that we can convert into a UTC timestamp. One way is to find the +// offset of now in the zone. The actual date may have a different offset (for +// example, if we handle a date in June while we're in December in a zone that +// observes DST), but we can check and adjust that. +// +// When handling many dates, calculating the offset for now every time is +// expensive. It's just a guess, so we can cache the offset to use even if we +// are right on a time change boundary (we'll just correct in the other +// direction). Using a timestamp from first read is a slight optimization for +// handling dates close to the current date, since those dates will usually be +// in the same offset (we could set the timestamp statically, instead). We use a +// single timestamp for all zones to make things a bit more predictable. +// +// This is safe for quickDT (used by local() and utc()) because we don't fill in +// higher-order units from tsNow (as we do in fromObject, this requires that +// offset is calculated from tsNow). +function guessOffsetForZone(zone) { + if (!DateTime._zoneOffsetGuessCache[zone]) { + if (DateTime._zoneOffsetTs === undefined) { + DateTime._zoneOffsetTs = Settings.now(); + } + + DateTime._zoneOffsetGuessCache[zone] = zone.offset(DateTime._zoneOffsetTs); + } + return DateTime._zoneOffsetGuessCache[zone]; +} + // this is a dumbed down version of fromObject() that runs about 60% faster // but doesn't do any validation, makes a bunch of assumptions about what units // are present, and so on. function quickDT(obj, opts) { const zone = normalizeZone(opts.zone, Settings.defaultZone), - loc = Locale.fromObject(opts), - tsNow = Settings.now(); + loc = Locale.fromObject(opts); let ts, o; @@ -392,10 +421,10 @@ function quickDT(obj, opts) { return DateTime.invalid(invalid); } - const offsetProvis = zone.offset(tsNow); + const offsetProvis = guessOffsetForZone(zone); [ts, o] = objToTS(obj, offsetProvis, zone); } else { - ts = tsNow; + ts = Settings.now(); } return new DateTime({ ts, zone, loc, o }); @@ -529,6 +558,22 @@ export default class DateTime { this.isLuxonDateTime = true; } + /** + * Timestamp to use for cached zone offset guesses (exposed for test) + * + * @access private + */ + static _zoneOffsetTs; + /** + * Cache for zone offset guesses (exposed for test). + * + * This optimizes quickDT via guessOffsetForZone to avoid repeated calls of + * zone.offset(). + * + * @access private + */ + static _zoneOffsetGuessCache = {}; + // CONSTRUCT /** diff --git a/test/datetime/dst.test.js b/test/datetime/dst.test.js index b28aa051d..c7c152558 100644 --- a/test/datetime/dst.test.js +++ b/test/datetime/dst.test.js @@ -2,111 +2,194 @@ import { DateTime, Settings } from "../../src/luxon"; -const local = (year, month, day, hour) => - DateTime.fromObject({ year, month, day, hour }, { zone: "America/New_York" }); - -test("Hole dates are bumped forward", () => { - const d = local(2017, 3, 12, 2); - expect(d.hour).toBe(3); - expect(d.offset).toBe(-4 * 60); -}); - -// this is questionable behavior, but I wanted to document it -test("Ambiguous dates pick the one with the current offset", () => { - const oldSettings = Settings.now; - try { - Settings.now = () => 1495653314595; // May 24, 2017 - let d = local(2017, 11, 5, 1); - expect(d.hour).toBe(1); - expect(d.offset).toBe(-4 * 60); - - Settings.now = () => 1484456400000; // Jan 15, 2017 - d = local(2017, 11, 5, 1); - expect(d.hour).toBe(1); - expect(d.offset).toBe(-5 * 60); - } finally { - Settings.now = oldSettings; - } -}); - -test("Adding an hour to land on the Spring Forward springs forward", () => { - const d = local(2017, 3, 12, 1).plus({ hour: 1 }); - expect(d.hour).toBe(3); - expect(d.offset).toBe(-4 * 60); -}); - -test("Subtracting an hour to land on the Spring Forward springs forward", () => { - const d = local(2017, 3, 12, 3).minus({ hour: 1 }); - expect(d.hour).toBe(1); - expect(d.offset).toBe(-5 * 60); -}); - -test("Adding an hour to land on the Fall Back falls back", () => { - const d = local(2017, 11, 5, 0).plus({ hour: 2 }); - expect(d.hour).toBe(1); - expect(d.offset).toBe(-5 * 60); -}); - -test("Subtracting an hour to land on the Fall Back falls back", () => { - let d = local(2017, 11, 5, 3).minus({ hour: 2 }); - expect(d.hour).toBe(1); - expect(d.offset).toBe(-5 * 60); - - d = d.minus({ hour: 1 }); - expect(d.hour).toBe(1); - expect(d.offset).toBe(-4 * 60); -}); - -test("Changing a calendar date to land on a hole bumps forward", () => { - let d = local(2017, 3, 11, 2).plus({ day: 1 }); - expect(d.hour).toBe(3); - expect(d.offset).toBe(-4 * 60); - - d = local(2017, 3, 13, 2).minus({ day: 1 }); - expect(d.hour).toBe(3); - expect(d.offset).toBe(-4 * 60); -}); - -test("Changing a calendar date to land on an ambiguous time chooses the closest one", () => { - let d = local(2017, 11, 4, 1).plus({ day: 1 }); - expect(d.hour).toBe(1); - expect(d.offset).toBe(-4 * 60); - - d = local(2017, 11, 6, 1).minus({ day: 1 }); - expect(d.hour).toBe(1); - expect(d.offset).toBe(-5 * 60); -}); - -test("Start of a 0:00->1:00 DST day is 1:00", () => { - const d = DateTime.fromObject( - { - year: 2017, - month: 10, - day: 15, - }, - { - zone: "America/Sao_Paulo", +const dateTimeConstructors = { + fromObject: (year, month, day, hour) => + DateTime.fromObject({ year, month, day, hour }, { zone: "America/New_York" }), + local: (year, month, day, hour) => + DateTime.local(year, month, day, hour, { zone: "America/New_York" }), +}; + +for (const [name, local] of Object.entries(dateTimeConstructors)) { + describe(`DateTime.${name}`, () => { + test("Hole dates are bumped forward", () => { + const d = local(2017, 3, 12, 2); + expect(d.hour).toBe(3); + expect(d.offset).toBe(-4 * 60); + }); + + if (name == "fromObject") { + // this is questionable behavior, but I wanted to document it + test("Ambiguous dates pick the one with the current offset", () => { + const oldSettings = Settings.now; + try { + Settings.now = () => 1495653314595; // May 24, 2017 + let d = local(2017, 11, 5, 1); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-4 * 60); + + Settings.now = () => 1484456400000; // Jan 15, 2017 + d = local(2017, 11, 5, 1); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-5 * 60); + } finally { + Settings.now = oldSettings; + } + }); + } else { + test("Ambiguous dates pick the one with the cached offset", () => { + const oldSettings = Settings.now; + try { + DateTime._zoneOffsetGuessCache = {}; + DateTime._zoneOffsetTs = undefined; + Settings.now = () => 1495653314595; // May 24, 2017 + let d = local(2017, 11, 5, 1); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-4 * 60); + + Settings.now = () => 1484456400000; // Jan 15, 2017 + d = local(2017, 11, 5, 1); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-4 * 60); + + DateTime._zoneOffsetGuessCache = {}; + DateTime._zoneOffsetTs = undefined; + + Settings.now = () => 1484456400000; // Jan 15, 2017 + d = local(2017, 11, 5, 1); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-5 * 60); + + Settings.now = () => 1495653314595; // May 24, 2017 + d = local(2017, 11, 5, 1); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-5 * 60); + } finally { + Settings.now = oldSettings; + } + }); } - ).startOf("day"); - expect(d.day).toBe(15); - expect(d.hour).toBe(1); - expect(d.minute).toBe(0); - expect(d.second).toBe(0); -}); -test("End of a 0:00->1:00 DST day is 23:59", () => { - const d = DateTime.fromObject( - { - year: 2017, - month: 10, - day: 15, - }, - { - zone: "America/Sao_Paulo", + test("Adding an hour to land on the Spring Forward springs forward", () => { + const d = local(2017, 3, 12, 1).plus({ hour: 1 }); + expect(d.hour).toBe(3); + expect(d.offset).toBe(-4 * 60); + }); + + test("Subtracting an hour to land on the Spring Forward springs forward", () => { + const d = local(2017, 3, 12, 3).minus({ hour: 1 }); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-5 * 60); + }); + + test("Adding an hour to land on the Fall Back falls back", () => { + const d = local(2017, 11, 5, 0).plus({ hour: 2 }); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-5 * 60); + }); + + test("Subtracting an hour to land on the Fall Back falls back", () => { + let d = local(2017, 11, 5, 3).minus({ hour: 2 }); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-5 * 60); + + d = d.minus({ hour: 1 }); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-4 * 60); + }); + + test("Changing a calendar date to land on a hole bumps forward", () => { + let d = local(2017, 3, 11, 2).plus({ day: 1 }); + expect(d.hour).toBe(3); + expect(d.offset).toBe(-4 * 60); + + d = local(2017, 3, 13, 2).minus({ day: 1 }); + expect(d.hour).toBe(3); + expect(d.offset).toBe(-4 * 60); + }); + + test("Changing a calendar date to land on an ambiguous time chooses the closest one", () => { + let d = local(2017, 11, 4, 1).plus({ day: 1 }); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-4 * 60); + + d = local(2017, 11, 6, 1).minus({ day: 1 }); + expect(d.hour).toBe(1); + expect(d.offset).toBe(-5 * 60); + }); + + test("Start of a 0:00->1:00 DST day is 1:00", () => { + const d = DateTime.fromObject( + { + year: 2017, + month: 10, + day: 15, + }, + { + zone: "America/Sao_Paulo", + } + ).startOf("day"); + expect(d.day).toBe(15); + expect(d.hour).toBe(1); + expect(d.minute).toBe(0); + expect(d.second).toBe(0); + }); + + test("End of a 0:00->1:00 DST day is 23:59", () => { + const d = DateTime.fromObject( + { + year: 2017, + month: 10, + day: 15, + }, + { + zone: "America/Sao_Paulo", + } + ).endOf("day"); + expect(d.day).toBe(15); + expect(d.hour).toBe(23); + expect(d.minute).toBe(59); + expect(d.second).toBe(59); + }); + }); +} + +describe("DateTime.local() with offset caching", () => { + const edtTs = 1495653314000; // May 24, 2017 15:15:14 -0400 + const estTs = 1484456400000; // Jan 15, 2017 00:00 -0500 + + const edtDate = [2017, 5, 24, 15, 15, 14, 0]; + const estDate = [2017, 1, 15, 0, 0, 0, 0]; + + const timestamps = { EDT: edtTs, EST: estTs }; + const dates = { EDT: edtDate, EST: estDate }; + const zoneObj = { zone: "America/New_York" }; + + for (const [cacheName, cacheTs] of Object.entries(timestamps)) { + for (const [nowName, nowTs] of Object.entries(timestamps)) { + for (const [dateName, date] of Object.entries(dates)) { + test(`cache = ${cacheName}, now = ${nowName}, date = ${dateName}`, () => { + const oldSettings = Settings.now; + try { + Settings.now = () => cacheTs; + DateTime._zoneOffsetGuessCache = {}; + DateTime._zoneOffsetTs = undefined; + // load cache + DateTime.local(2020, 1, 1, 0, zoneObj); + + Settings.now = () => nowTs; + const dt = DateTime.local(...date, zoneObj); + expect(dt.toMillis()).toBe(timestamps[dateName]); + expect(dt.year).toBe(date[0]); + expect(dt.month).toBe(date[1]); + expect(dt.day).toBe(date[2]); + expect(dt.hour).toBe(date[3]); + expect(dt.minute).toBe(date[4]); + expect(dt.second).toBe(date[5]); + } finally { + Settings.now = oldSettings; + } + }); + } } - ).endOf("day"); - expect(d.day).toBe(15); - expect(d.hour).toBe(23); - expect(d.minute).toBe(59); - expect(d.second).toBe(59); + } }); diff --git a/test/datetime/regexParse.test.js b/test/datetime/regexParse.test.js index db8ee5302..db84b968f 100644 --- a/test/datetime/regexParse.test.js +++ b/test/datetime/regexParse.test.js @@ -1,6 +1,7 @@ /* global test expect */ import { DateTime } from "../../src/luxon"; +import { withNow } from "../helpers"; //------ // .fromISO @@ -624,6 +625,28 @@ test("DateTime.fromISO() accepts extended zones on bare times", () => { }); }); +withNow( + "DateTime.fromISO() accepts extended zones on bare times when UTC and zone are in different days", + DateTime.fromISO("2023-11-20T23:30:00.000Z"), + () => { + const { year, month, day } = DateTime.now().setZone("Europe/Paris"); + let dt = DateTime.fromISO("10:23:54[Europe/Paris]", { + setZone: true, + }); + expect(dt.isValid).toBe(true); + expect(dt.zoneName).toBe("Europe/Paris"); + expect(dt.toObject()).toEqual({ + year, + month, + day, + hour: 10, + minute: 23, + second: 54, + millisecond: 0, + }); + } +); + test("DateTime.fromISO() accepts some technically incorrect stuff", () => { // these are formats that aren't technically valid but we parse anyway. // Testing them more to document them than anything else From 3e363ee6a729735c8ec5cdf52827ff2840d4337d Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Fri, 22 Dec 2023 13:30:40 -0500 Subject: [PATCH 5/8] Memoize digitsRegex Profiles reveal that a substantial portion of fromFormat time is spent in digitsRegex. It is only called from unitsForToken and it is always called with 11 suffixes to match different numbers of digits. There are 21 numbering systems, so a fully expanded cache would hold 11*21 = 231 regexes. Benchmarks show about a 1.5x improvement on DateTime.fromFormat --- src/impl/digits.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/impl/digits.js b/src/impl/digits.js index 6ac1bae05..9ae21b02d 100644 --- a/src/impl/digits.js +++ b/src/impl/digits.js @@ -70,6 +70,18 @@ export function parseDigits(str) { } } +// cache of {numberingSystem: {append: regex}} +const digitRegexCache = {}; + export function digitRegex({ numberingSystem }, append = "") { - return new RegExp(`${numberingSystems[numberingSystem || "latn"]}${append}`); + const ns = numberingSystem || "latn"; + + if (!digitRegexCache[ns]) { + digitRegexCache[ns] = {}; + } + if (!digitRegexCache[ns][append]) { + digitRegexCache[ns][append] = new RegExp(`${numberingSystems[ns]}${append}`); + } + + return digitRegexCache[ns][append]; } From b56224e0a7f5cfa402750cfd3a96e911f8230157 Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Fri, 22 Dec 2023 14:08:24 -0500 Subject: [PATCH 6/8] Add DateTime.buildFormatParser and DateTime.fromFormatParser This allows constructing a parser for a locale/format and reusing it when parsing dates. Without this, DateTime.fromFormat constructs a new parser on every call. When parsing large amounts of date strings, this gets rather slow. In benchmarks, this speeds up parsing by 4.4x --- benchmarks/datetime.js | 10 +++++ src/datetime.js | 69 ++++++++++++++++++++++++++++++++ src/impl/locale.js | 4 ++ src/impl/tokenParser.js | 68 ++++++++++++++++++++++--------- test/datetime/tokenParse.test.js | 25 ++++++++++++ 5 files changed, 158 insertions(+), 18 deletions(-) diff --git a/benchmarks/datetime.js b/benchmarks/datetime.js index 39d6bf07c..32839ad2a 100644 --- a/benchmarks/datetime.js +++ b/benchmarks/datetime.js @@ -8,6 +8,8 @@ function runDateTimeSuite() { const dt = DateTime.now(); + const formatParser = DateTime.buildFormatParser("yyyy/MM/dd HH:mm:ss.SSS"); + suite .add("DateTime.local", () => { DateTime.now(); @@ -32,6 +34,14 @@ function runDateTimeSuite() { zone: "America/Los_Angeles", }); }) + .add("DateTime.fromFormatParser", () => { + DateTime.fromFormatParser("1982/05/25 09:10:11.445", formatParser); + }) + .add("DateTime.fromFormatParser with zone", () => { + DateTime.fromFormatParser("1982/05/25 09:10:11.445", formatParser, { + zone: "America/Los_Angeles", + }); + }) .add("DateTime#setZone", () => { dt.setZone("America/Los_Angeles"); }) diff --git a/src/datetime.js b/src/datetime.js index 9472cb531..111faf02b 100644 --- a/src/datetime.js +++ b/src/datetime.js @@ -28,6 +28,7 @@ import { explainFromTokens, formatOptsToTokens, expandMacroTokens, + TokenParser, } from "./impl/tokenParser.js"; import { gregorianToWeek, @@ -2229,6 +2230,74 @@ export default class DateTime { return DateTime.fromFormatExplain(text, fmt, options); } + /** + * Build a parser for `fmt` using the given locale. This parser can be passed + * to {@link DateTime.fromFormatParser} to a parse a date in this format. This + * can be used to optimize cases where many dates need to be parsed in a + * specific format. + * + * @param {String} fmt - the format the string is expected to be in (see + * description) + * @param {Object} options - options used to set locale and numberingSystem + * for parser + * @returns {TokenParser} - opaque object to be used + */ + static buildFormatParser(fmt, options = {}) { + const { locale = null, numberingSystem = null } = options, + localeToUse = Locale.fromOpts({ + locale, + numberingSystem, + defaultToEN: true, + }); + return new TokenParser(localeToUse, fmt); + } + + /** + * Create a DateTime from an input string and format parser. + * + * The format parser must have been created with the same locale as this call. + * + * @param {String} text - the string to parse + * @param {TokenParser} formatParser - parser from {@link DateTime.buildFormatParser} + * @param {Object} opts - options taken by fromFormat() + * @returns {DateTime} + */ + static fromFormatParser(text, formatParser, opts = {}) { + if (isUndefined(text) || isUndefined(formatParser)) { + throw new InvalidArgumentError( + "fromFormatParser requires an input string and a format parser" + ); + } + const { locale = null, numberingSystem = null } = opts, + localeToUse = Locale.fromOpts({ + locale, + numberingSystem, + defaultToEN: true, + }); + + if (!localeToUse.equals(formatParser.locale)) { + throw new InvalidArgumentError( + `fromFormatParser called with a locale of ${localeToUse}, ` + + `but the format parser was created for ${formatParser.locale}` + ); + } + + const { result, zone, specificOffset, invalidReason } = formatParser.explainFromTokens(text); + + if (invalidReason) { + return DateTime.invalid(invalidReason); + } else { + return parseDataToDateTime( + result, + zone, + opts, + `format ${formatParser.format}`, + text, + specificOffset + ); + } + } + // FORMAT PRESETS /** diff --git a/src/impl/locale.js b/src/impl/locale.js index f1caf1495..cd55b3bfc 100644 --- a/src/impl/locale.js +++ b/src/impl/locale.js @@ -539,4 +539,8 @@ export default class Locale { this.outputCalendar === other.outputCalendar ); } + + toString() { + return `Locale(${this.locale}, ${this.numberingSystem}, ${this.outputCalendar})`; + } } diff --git a/src/impl/tokenParser.js b/src/impl/tokenParser.js index 8dd38f37f..48a7595ed 100644 --- a/src/impl/tokenParser.js +++ b/src/impl/tokenParser.js @@ -432,27 +432,59 @@ export function expandMacroTokens(tokens, locale) { * @private */ -export function explainFromTokens(locale, input, format) { - const tokens = expandMacroTokens(Formatter.parseFormat(format), locale), - units = tokens.map((t) => unitForToken(t, locale)), - disqualifyingUnit = units.find((t) => t.invalidReason); +export class TokenParser { + constructor(locale, format) { + this.locale = locale; + this.format = format; + this.tokens = expandMacroTokens(Formatter.parseFormat(format), locale); + this.units = this.tokens.map((t) => unitForToken(t, locale)); + this.disqualifyingUnit = this.units.find((t) => t.invalidReason); + + if (!this.disqualifyingUnit) { + const [regexString, handlers] = buildRegex(this.units); + this.regex = RegExp(regexString, "i"); + this.handlers = handlers; + } + } - if (disqualifyingUnit) { - return { input, tokens, invalidReason: disqualifyingUnit.invalidReason }; - } else { - const [regexString, handlers] = buildRegex(units), - regex = RegExp(regexString, "i"), - [rawMatches, matches] = match(input, regex, handlers), - [result, zone, specificOffset] = matches - ? dateTimeFromMatches(matches) - : [null, null, undefined]; - if (hasOwnProperty(matches, "a") && hasOwnProperty(matches, "H")) { - throw new ConflictingSpecificationError( - "Can't include meridiem when specifying 24-hour format" - ); + explainFromTokens(input) { + if (!this.isValid) { + return { input, tokens: this.tokens, invalidReason: this.invalidReason }; + } else { + const [rawMatches, matches] = match(input, this.regex, this.handlers), + [result, zone, specificOffset] = matches + ? dateTimeFromMatches(matches) + : [null, null, undefined]; + if (hasOwnProperty(matches, "a") && hasOwnProperty(matches, "H")) { + throw new ConflictingSpecificationError( + "Can't include meridiem when specifying 24-hour format" + ); + } + return { + input, + tokens: this.tokens, + regex: this.regex, + rawMatches, + matches, + result, + zone, + specificOffset, + }; } - return { input, tokens, regex, rawMatches, matches, result, zone, specificOffset }; } + + get isValid() { + return !this.disqualifyingUnit; + } + + get invalidReason() { + return this.disqualifyingUnit ? this.disqualifyingUnit.invalidReason : null; + } +} + +export function explainFromTokens(locale, input, format) { + const parser = new TokenParser(locale, format); + return parser.explainFromTokens(input); } export function parseFromTokens(locale, input, format) { diff --git a/test/datetime/tokenParse.test.js b/test/datetime/tokenParse.test.js index 4025821d8..8b5c6a8d7 100644 --- a/test/datetime/tokenParse.test.js +++ b/test/datetime/tokenParse.test.js @@ -1224,3 +1224,28 @@ test("DateTime.expandFormat respects the hour cycle when forced by the macro tok const format = DateTime.expandFormat("T", { locale: "en-US" }); expect(format).toBe("H:m"); }); + +//------ +// .fromFormatParser +//------- + +test("DateTime.fromFormatParser behaves equivalently to DateTime.fromFormat", () => { + const dateTimeStr = "1982/05/25 09:10:11.445"; + const format = "yyyy/MM/dd HH:mm:ss.SSS"; + const formatParser = DateTime.buildFormatParser(format); + const ff1 = DateTime.fromFormat(dateTimeStr, format), + ffP1 = DateTime.fromFormatParser(dateTimeStr, formatParser); + + expect(ffP1).toEqual(ff1); + expect(ffP1.isValid).toBe(true); +}); + +test("DateTime.fromFormatParser throws error when used with a different locale than it was created with", () => { + const format = "yyyy/MM/dd HH:mm:ss.SSS"; + const formatParser = DateTime.buildFormatParser(format, { locale: "es-ES" }); + expect(() => + DateTime.fromFormatParser("1982/05/25 09:10:11.445", formatParser, { locale: "es-MX" }) + ).toThrowError( + "fromFormatParser called with a locale of Locale(es-MX, null, null), but the format parser was created for Locale(es-ES, null, null)" + ); +}); From c7e40aceec56f0062e5354f4b209e6913fed6de5 Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Mon, 27 Nov 2023 17:52:11 -0500 Subject: [PATCH 7/8] Use hackyOffset if it parses date strings in the expected format The time zone offset of a date can be computed on platforms that support it (anything that's not super ancient) by using Intl.DateTimeFormat.formatToParts with en-US to output an array of the date components. For legacy reasons, you can also generate a date string using Intl.DateTimeFormat.format which can be parsed into an array using regexes. The string/regex approach (hackyOffset) is way faster (2-4x), but much more susceptible to weird client configurations. This detects whether hackyOffset is able to parse a known date correctly, and uses it if it does. --- src/zones/IANAZone.js | 57 ++++++++++++++++++++++++++++++++++++++--- test/zones/IANA.test.js | 37 +++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/zones/IANAZone.js b/src/zones/IANAZone.js index ad5945132..d7036d7ef 100644 --- a/src/zones/IANAZone.js +++ b/src/zones/IANAZone.js @@ -29,11 +29,21 @@ const typeToPos = { second: 6, }; +function offsetComponentsFromDtf(dtf, date) { + if (IANAZone.hackyOffsetParsesCorrectly()) { + return hackyOffset(dtf, date); + } else if (dtf.formatToParts) { + return partsOffset(dtf, date); + } else { + throw new Error("Unable to compute time zone offset using Intl.DateTimeFormat"); + } +} + function hackyOffset(dtf, date) { const formatted = dtf.format(date).replace(/\u200E/g, ""), parsed = /(\d+)\/(\d+)\/(\d+) (AD|BC),? (\d+):(\d+):(\d+)/.exec(formatted), [, fMonth, fDay, fYear, fadOrBc, fHour, fMinute, fSecond] = parsed; - return [fYear, fMonth, fDay, fadOrBc, fHour, fMinute, fSecond]; + return [+fYear, +fMonth, +fDay, fadOrBc, +fHour, +fMinute, +fSecond]; } function partsOffset(dtf, date) { @@ -52,6 +62,8 @@ function partsOffset(dtf, date) { return filled; } +let hackyOffsetParsesCorrectly = undefined; + let ianaZoneCache = {}; /** * A zone identified by an IANA identifier, like America/New_York @@ -76,6 +88,45 @@ export default class IANAZone extends Zone { static resetCache() { ianaZoneCache = {}; dtfCache = {}; + hackyOffsetParsesCorrectly = undefined; + } + + /** + * Get a DTF instance from the cache. Should only be used for testing. + * + * @access private + */ + static getDtf(zone) { + return makeDTF(zone); + } + + /** + * Returns whether hackyOffset works correctly for a known date. If it does, + * we can use hackyOffset which is faster. + * @returns {boolean} + */ + static hackyOffsetParsesCorrectly() { + if (hackyOffsetParsesCorrectly === undefined) { + const dtf = makeDTF("UTC"); + try { + const [year, month, day, adOrBc, hour, minute, second] = hackyOffset( + dtf, + // arbitrary date + new Date(Date.UTC(1969, 11, 31, 15, 45, 55)) + ); + hackyOffsetParsesCorrectly = + year === 1969 && + month === 12 && + day === 31 && + adOrBc === "AD" && + hour === 15 && + minute === 45 && + second === 55; + } catch { + hackyOffsetParsesCorrectly = false; + } + } + return hackyOffsetParsesCorrectly; } /** @@ -150,9 +201,7 @@ export default class IANAZone extends Zone { if (isNaN(date)) return NaN; const dtf = makeDTF(this.name); - let [year, month, day, adOrBc, hour, minute, second] = dtf.formatToParts - ? partsOffset(dtf, date) - : hackyOffset(dtf, date); + let [year, month, day, adOrBc, hour, minute, second] = offsetComponentsFromDtf(dtf, date); if (adOrBc === "BC") { year = -Math.abs(year) + 1; diff --git a/test/zones/IANA.test.js b/test/zones/IANA.test.js index e42ac94cf..545bb495e 100644 --- a/test/zones/IANA.test.js +++ b/test/zones/IANA.test.js @@ -1,4 +1,4 @@ -/* global test expect */ +/* global test expect jest */ import { FixedOffsetZone, IANAZone } from "../../src/luxon"; test("IANAZone.create returns a singleton per zone name", () => { @@ -16,6 +16,41 @@ test("IANAZone.create should return IANAZone instance", () => { expect(result).toBeInstanceOf(IANAZone); }); +describe("IANAZone.hackyOffsetParsesCorrectly", () => { + beforeEach(() => { + IANAZone.resetCache(); + }); + + test("is true", () => { + expect(IANAZone.hackyOffsetParsesCorrectly()).toBe(true); + }); + + test("is true when the date format is as expected", () => { + jest + .spyOn(IANAZone.getDtf("UTC"), "format") + .mockImplementation(() => "12/31/1969 AD, 15:45:55"); + expect(IANAZone.hackyOffsetParsesCorrectly()).toBe(true); + }); + + test("is false when the date format swaps the month and day", () => { + jest + .spyOn(IANAZone.getDtf("UTC"), "format") + .mockImplementation(() => "31/12/1969 AD, 15:45:55"); + expect(IANAZone.hackyOffsetParsesCorrectly()).toBe(false); + }); + + test("is false when the date format uses different delimiters", () => { + jest + .spyOn(IANAZone.getDtf("UTC"), "format") + .mockImplementation(() => "12-31-1969 AD, 15:45:55"); + expect(IANAZone.hackyOffsetParsesCorrectly()).toBe(false); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); +}); + test("IANAZone.isValidSpecifier", () => { expect(IANAZone.isValidSpecifier("America/New_York")).toBe(true); // this used to return true but now returns false, because we just defer to isValidZone From 87fd3ff3823eae941c1995735c2a4431a31ce1b5 Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Tue, 9 Jan 2024 18:51:46 -0500 Subject: [PATCH 8/8] Run buildAll in prepare, move husky install to postinstall Husky adds git hooks into the repo. We want the dev experience of $ git clone remote/luxon $ npm install # installs husky hooks $ ...develop develop develop... $ git commit _lint runs, everything works_ We also want to build luxon when we call npm pack or npm publish. We also want to build luxon when it is installed somewhere else as a git dependency (https://docs.npmjs.com/cli/v10/configuring-npm/package-json#git-urls-as-dependencies) Git dependencies work by running `npm install` from GitFetcher and then depend on DirFetcher running `npm prepare` (but only prepare, not prepack or postpack) This change moves `husky install` to postinstall (which will, unfortunately, still pointlessly run when we install luxon as a git dependency). It moves the buildAll to prepare from prepack so that the build happens when installed as a git dependency. This should preserve existing behavior while enabling installation from git. When installing from git and/or in CI environments, the husky command is sometimes not available in postinstall or is available but bails out when not run in the context of a git repo. --- package-lock.json | 1 + package.json | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index e7168fc7f..34b58de07 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,6 +7,7 @@ "": { "name": "luxon", "version": "3.4.4", + "hasInstallScript": true, "license": "MIT", "devDependencies": { "@babel/core": "^7.18.6", diff --git a/package.json b/package.json index 7f97f3dd5..37cc54b11 100644 --- a/package.json +++ b/package.json @@ -28,8 +28,8 @@ "format-check": "prettier --check 'src/**/*.js' 'test/**/*.js' 'benchmarks/*.js'", "benchmark": "node benchmarks/index.js", "codecov": "codecov", - "prepack": "babel-node tasks/buildAll.js", - "prepare": "husky install", + "postinstall": "husky install || exit 0", + "prepare": "babel-node tasks/buildAll.js", "show-site": "http-server build" }, "lint-staged": {