Repository: moment/luxon
Pull request #1108 (2022-01-02 07:20:08)

Continuation of #1097
@gpaucot Added some commits:

 1. I really wanted to push the work into `toSQLDate` and `toSQLTime`. So I set up both your fully-inlined one and my refactor and benchmarked them against each other, then tweaked mine until it was within the benchmark's noise level.
 2. Admittedly, my code benefited from some tweaks to the logic, and once I backported those into the inlined code, that code got faster again. But only by a little, so I'm going to live with it.
 3. I also removed some no-longer-needed code and did some other cleanup

These were the logic changes relative to your PR:

 1. Turns out, we're supposed to always use 6-digit years for negative years. That's actually #951. That has a small perf advantage because it's quicker to compare `o.c.year > 9999 || o.c.year < 0` than it is to do `Math.abs(o.c.year) > 9999`. So we'll squash that bug by-the-by.
 2. You had logic for `allowZ`, but that's not actually an option on `toISO`. You always get Z for the UTC zone; the old code hardcoded into the call to the formatter. No real perf difference either way


Patch:
diff --git a/benchmarks/datetime.js b/benchmarks/datetime.js
index 1fcd0faf..0362f8ee 100644
--- a/benchmarks/datetime.js
+++ b/benchmarks/datetime.js
@@ -24,11 +24,11 @@ function runDateTimeSuite() {
       .add("DateTime.fromSQL", () => {
         DateTime.fromSQL("2016-05-14 10:23:54.2346");
       })
-      .add("DateTime.fromString", () => {
-        DateTime.fromString("1982/05/25 09:10:11.445", "yyyy/MM/dd HH:mm:ss.SSS");
+      .add("DateTime.fromFormat", () => {
+        DateTime.fromFormat("1982/05/25 09:10:11.445", "yyyy/MM/dd HH:mm:ss.SSS");
       })
-      .add("DateTime.fromString with zone", () => {
-        DateTime.fromString("1982/05/25 09:10:11.445", "yyyy/MM/dd HH:mm:ss.SSS", {
+      .add("DateTime.fromFormat with zone", () => {
+        DateTime.fromFormat("1982/05/25 09:10:11.445", "yyyy/MM/dd HH:mm:ss.SSS", {
           zone: "America/Los_Angeles",
         });
       })
diff --git a/src/datetime.js b/src/datetime.js
index 9d396b0e..625288a2 100644
--- a/src/datetime.js
+++ b/src/datetime.js
@@ -18,6 +18,7 @@ import {
   normalizeObject,
   roundTo,
   objToLocalTS,
+  padStart,
 } from "./impl/util.js";
 import { normalizeZone } from "./impl/zoneUtil.js";
 import diff from "./impl/diff.js";
@@ -188,46 +189,61 @@ function toTechFormat(dt, format, allowZ = true) {
     : null;
 }
 
-// technical time formats (e.g. the time part of ISO 8601), take some options
-// and this commonizes their handling
-function toTechTimeFormat(
-  dt,
-  {
-    suppressSeconds = false,
-    suppressMilliseconds = false,
-    includeOffset,
-    includePrefix = false,
-    includeZone = false,
-    spaceZone = false,
-    format = "extended",
+function toISODate(o, extended) {
+  const longFormat = o.c.year > 9999 || o.c.year < 0;
+  let c = "";
+  if (longFormat && o.c.year >= 0) c += "+";
+  c += padStart(o.c.year, longFormat ? 6 : 4);
+
+  if (extended) {
+    c += "-";
+    c += padStart(o.c.month);
+    c += "-";
+    c += padStart(o.c.day);
+  } else {
+    c += padStart(o.c.month);
+    c += padStart(o.c.day);
   }
-) {
-  let fmt = format === "basic" ? "HHmm" : "HH:mm";
+  return c;
+}
 
-  if (!suppressSeconds || dt.second !== 0 || dt.millisecond !== 0) {
-    fmt += format === "basic" ? "ss" : ":ss";
-    if (!suppressMilliseconds || dt.millisecond !== 0) {
-      fmt += ".SSS";
+function toISOTime(o, extended, suppressSeconds, suppressMilliseconds, includeOffset) {
+  let c = padStart(o.c.hour);
+  if (extended) {
+    c += ":";
+    c += padStart(o.c.minute);
+    if (o.c.second !== 0 || !suppressSeconds) {
+      c += ":";
     }
+  } else {
+    c += padStart(o.c.minute);
   }
 
-  if ((includeZone || includeOffset) && spaceZone) {
-    fmt += " ";
-  }
+  if (o.c.second !== 0 || !suppressSeconds) {
+    c += padStart(o.c.second);
 
-  if (includeZone) {
-    fmt += "z";
-  } else if (includeOffset) {
-    fmt += format === "basic" ? "ZZZ" : "ZZ";
+    if (o.c.millisecond !== 0 || !suppressMilliseconds) {
+      c += ".";
+      c += padStart(o.c.millisecond, 3);
+    }
   }
 
-  let str = toTechFormat(dt, fmt);
-
-  if (includePrefix) {
-    str = "T" + str;
+  if (includeOffset) {
+    if (o.isOffsetFixed && o.offset === 0) {
+      c += "Z";
+    } else if (o.o < 0) {
+      c += "-";
+      c += padStart(Math.trunc(-o.o / 60));
+      c += ":";
+      c += padStart(Math.trunc(-o.o % 60));
+    } else {
+      c += "+";
+      c += padStart(Math.trunc(o.o / 60));
+      c += ":";
+      c += padStart(Math.trunc(o.o % 60));
+    }
   }
-
-  return str;
+  return c;
 }
 
 // defaults for unspecified units in the supported calendars
@@ -1394,7 +1410,7 @@ export default class DateTime {
    * See {@link DateTime#plus}
    * @param {Duration|Object|number} duration - The amount to subtract. Either a Luxon Duration, a number of milliseconds, the object argument to Duration.fromObject()
    @return {DateTime}
-  */
+   */
   minus(duration) {
     if (!this.isValid) return this;
     const dur = Duration.fromDurationLike(duration).negate();
@@ -1548,12 +1564,22 @@ export default class DateTime {
    * @example DateTime.now().toISO({ format: 'basic' }) //=> '20170422T204705.335-0400'
    * @return {string}
    */
-  toISO(opts = {}) {
+  toISO({
+    format = "extended",
+    suppressSeconds = false,
+    suppressMilliseconds = false,
+    includeOffset = true,
+  } = {}) {
     if (!this.isValid) {
       return null;
     }
 
-    return `${this.toISODate(opts)}T${this.toISOTime(opts)}`;
+    const ext = format === "extended";
+
+    let c = toISODate(this, ext);
+    c += "T";
+    c += toISOTime(this, ext, suppressSeconds, suppressMilliseconds, includeOffset);
+    return c;
   }
 
   /**
@@ -1565,12 +1591,11 @@ export default class DateTime {
    * @return {string}
    */
   toISODate({ format = "extended" } = {}) {
-    let fmt = format === "basic" ? "yyyyMMdd" : "yyyy-MM-dd";
-    if (this.year > 9999) {
-      fmt = "+" + fmt;
+    if (!this.isValid) {
+      return null;
     }
 
-    return toTechFormat(this, fmt);
+    return toISODate(this, format === "extended");
   }
 
   /**
@@ -1603,13 +1628,15 @@ export default class DateTime {
     includePrefix = false,
     format = "extended",
   } = {}) {
-    return toTechTimeFormat(this, {
-      suppressSeconds,
-      suppressMilliseconds,
-      includeOffset,
-      includePrefix,
-      format,
-    });
+    if (!this.isValid) {
+      return null;
+    }
+
+    let c = includePrefix ? "T" : "";
+    return (
+      c +
+      toISOTime(this, format === "extended", suppressSeconds, suppressMilliseconds, includeOffset)
+    );
   }
 
   /**
@@ -1640,7 +1667,10 @@ export default class DateTime {
    * @return {string}
    */
   toSQLDate() {
-    return toTechFormat(this, "yyyy-MM-dd");
+    if (!this.isValid) {
+      return null;
+    }
+    return toISODate(this, true);
   }
 
   /**
@@ -1655,11 +1685,18 @@ export default class DateTime {
    * @return {string}
    */
   toSQLTime({ includeOffset = true, includeZone = false } = {}) {
-    return toTechTimeFormat(this, {
-      includeOffset,
-      includeZone,
-      spaceZone: true,
-    });
+    let fmt = "HH:mm:ss.SSS";
+
+    if (includeZone || includeOffset) {
+      fmt += " ";
+      if (includeZone) {
+        fmt += "z";
+      } else if (includeOffset) {
+        fmt += "ZZ";
+      }
+    }
+
+    return toTechFormat(this, fmt, true);
   }
 
   /**
@@ -1824,8 +1861,8 @@ export default class DateTime {
     if (!this.isValid) return false;
 
     const inputMs = otherDateTime.valueOf();
-    const adjustedToZone = this.setZone(otherDateTime.zone, { keepLocalTime: true });
-    return adjustedToZone.startOf(unit) <= inputMs && inputMs <= adjustedToZone.endOf(unit);
+    const otherZoneDateTime = this.setZone(otherDateTime.zone, { keepLocalTime: true });
+    return otherZoneDateTime.startOf(unit) <= inputMs && inputMs <= otherZoneDateTime.endOf(unit);
   }
 
   /**
diff --git a/src/duration.js b/src/duration.js
index 5db8ddd8..fdd5ad5c 100644
--- a/src/duration.js
+++ b/src/duration.js
@@ -425,36 +425,6 @@ export default class Duration {
       : INVALID;
   }
 
-  /**
-   * Returns a string representation of a Duration with all units included
-   * To modify its behavior use the `listStyle` and any Intl.NumberFormat option, though `unitDisplay` is especially relevant. See {@link Intl.NumberFormat}.
-   * @param opts - On option object to override the formatting. Accepts the same keys as the options parameter of the native `Int.NumberFormat` constructor, as well as `listStyle`.
-   * @example
-   * ```js
-   * var dur = Duration.fromObject({ days: 1, hours: 5, minutes: 6 })
-   * dur.toHuman() //=> '1 day, 5 hours, 6 minutes'
-   * dur.toHuman({ listStyle: "long" }) //=> '1 day, 5 hours, and 6 minutes'
-   * dur.toHuman({ unitDisplay: "short" }) //=> '1 day, 5 hr, 6 min'
-   * ```
-   */
-  toHuman(opts = {}) {
-    const l = orderedUnits
-      .map((unit) => {
-        const val = this.values[unit];
-        if (isUndefined(val)) {
-          return null;
-        }
-        return this.loc
-          .numberFormatter({ style: "unit", unitDisplay: "long", ...opts, unit: unit.slice(0, -1) })
-          .format(val);
-      })
-      .filter((n) => n);
-
-    return this.loc
-      .listFormatter({ type: "conjunction", style: opts.listStyle || "narrow", ...opts })
-      .format(l);
-  }
-
   /**
    * Returns a JavaScript object with this Duration's values.
    * @example Duration.fromObject({ years: 1, days: 6, seconds: 2 }).toObject() //=> { years: 1, days: 6, seconds: 2 }
diff --git a/src/impl/locale.js b/src/impl/locale.js
index 257907b7..500a60c1 100644
--- a/src/impl/locale.js
+++ b/src/impl/locale.js
@@ -4,19 +4,6 @@ import Settings from "../settings.js";
 import DateTime from "../datetime.js";
 import IANAZone from "../zones/IANAZone.js";
 
-// todo - remap caching
-
-let intlLFCache = {};
-function getCachedLF(locString, opts = {}) {
-  const key = JSON.stringify([locString, opts]);
-  let dtf = intlLFCache[key];
-  if (!dtf) {
-    dtf = new Intl.ListFormat(locString, opts);
-    intlLFCache[key] = dtf;
-  }
-  return dtf;
-}
-
 let intlDTCache = {};
 function getCachedDTF(locString, opts = {}) {
   const key = JSON.stringify([locString, opts]);
@@ -157,10 +144,8 @@ class PolyNumberFormatter {
     this.padTo = opts.padTo || 0;
     this.floor = opts.floor || false;
 
-    const { padTo, floor, ...otherOpts } = opts;
-
-    if (!forceSimple || Object.keys(otherOpts).length > 0) {
-      const intlOpts = { useGrouping: false, ...opts };
+    if (!forceSimple) {
+      const intlOpts = { useGrouping: false };
       if (opts.padTo > 0) intlOpts.minimumIntegerDigits = opts.padTo;
       this.inf = getCachedINF(intl, intlOpts);
     }
@@ -323,7 +308,7 @@ export default class Locale {
     return this.fastNumbersCached;
   }
 
-  listingMode() {
+  listingMode(defaultOK = true) {
     const isActuallyEn = this.isEnglish();
     const hasNoWeirdness =
       (this.numberingSystem === null || this.numberingSystem === "latn") &&
@@ -436,10 +421,6 @@ export default class Locale {
     return new PolyRelFormatter(this.intl, this.isEnglish(), opts);
   }
 
-  listFormatter(opts = {}) {
-    return getCachedLF(this.intl, opts);
-  }
-
   isEnglish() {
     return (
       this.locale === "en" ||
diff --git a/src/impl/util.js b/src/impl/util.js
index 9f267211..43def24c 100644
--- a/src/impl/util.js
+++ b/src/impl/util.js
@@ -87,17 +87,14 @@ export function floorMod(x, n) {
 }
 
 export function padStart(input, n = 2) {
-  const minus = input < 0 ? "-" : "";
-  const target = minus ? input * -1 : input;
-  let result;
-
-  if (target.toString().length < n) {
-    result = ("0".repeat(n) + target).slice(-n);
+  const isNeg = input < 0;
+  let padded;
+  if (isNeg) {
+    padded = "-" + ("" + -input).padStart(n, "0");
   } else {
-    result = target.toString();
+    padded = ("" + input).padStart(n, "0");
   }
-
-  return `${minus}${result}`;
+  return padded;
 }
 
 export function parseInteger(string) {


Test patch:
diff --git a/test/datetime/format.test.js b/test/datetime/format.test.js
index 762d6e5c..2d4dd550 100644
--- a/test/datetime/format.test.js
+++ b/test/datetime/format.test.js
@@ -65,6 +65,26 @@ test("DateTime#toISO() rounds fractional timezone minute offsets", () => {
   );
 });
 
+test("DateTime#toISO() handles long gregorian format", () => {
+  const negativeYear = dt.set({ year: 12345 });
+  expect(negativeYear.toISO()).toBe("+012345-05-25T09:23:54.123Z");
+});
+
+test("DateTime#toISO() handles negative years", () => {
+  const negativeYear = dt.set({ year: -12345 });
+  expect(negativeYear.toISO()).toBe("-012345-05-25T09:23:54.123Z");
+});
+
+test("DateTime#toISO() default to Z when timezone is 00:00", () => {
+  const negativeYear = dt.setZone("utc");
+  expect(negativeYear.toISO()).toBe("1982-05-25T09:23:54.123Z");
+});
+
+test("DateTime#toISO() renders 00:00 for non-offset but non utc datetimes", () => {
+  const negativeYear = dt.setZone("Africa/Abidjan");
+  expect(negativeYear.toISO()).toBe("1982-05-25T09:23:54.123+00:00");
+});
+
 //------
 // #toISODate()
 //------
@@ -95,10 +115,10 @@ test("DateTime#toISODate() returns ISO 8601 date in format [±YYYYY]", () => {
 
 test("DateTime#toISODate() correctly pads negative years", () => {
   expect(DateTime.fromObject({ year: -1, month: 1, day: 1 }, { zone: "utc" }).toISODate()).toBe(
-    "-0001-01-01"
+    "-000001-01-01"
   );
   expect(DateTime.fromObject({ year: -10, month: 1, day: 1 }, { zone: "utc" }).toISODate()).toBe(
-    "-0010-01-01"
+    "-000010-01-01"
   );
 });
 
diff --git a/test/duration/format.test.js b/test/duration/format.test.js
index 686154fe..9a50c9d7 100644
--- a/test/duration/format.test.js
+++ b/test/duration/format.test.js
@@ -263,35 +263,3 @@ test("Duration#toFormat localizes the numbers", () => {
 test("Duration#toFormat returns a lame string for invalid durations", () => {
   expect(Duration.invalid("because").toFormat("yy")).toBe("Invalid Duration");
 });
-
-//------
-// #humanize()
-//------
-
-test("Duration#toHuman formats out a list", () => {
-  expect(dur().toHuman()).toEqual(
-    "1 year, 2 months, 1 week, 3 days, 4 hours, 5 minutes, 6 seconds, 7 milliseconds"
-  );
-});
-
-test("Duration#toHuman only shows the units you have", () => {
-  expect(Duration.fromObject({ years: 3, hours: 4 }).toHuman()).toEqual("3 years, 4 hours");
-});
-
-test("Duration#toHuman accepts a listStyle", () => {
-  expect(dur().toHuman({ listStyle: "long" })).toEqual(
-    "1 year, 2 months, 1 week, 3 days, 4 hours, 5 minutes, 6 seconds, and 7 milliseconds"
-  );
-});
-
-test("Duration#toHuman accepts number format opts", () => {
-  expect(dur().toHuman({ unitDisplay: "short" })).toEqual(
-    "1 yr, 2 mths, 1 wk, 3 days, 4 hr, 5 min, 6 sec, 7 ms"
-  );
-});
-
-test("Duration#toHuman works in differt languages", () => {
-  expect(dur().reconfigure({ locale: "fr" }).toHuman()).toEqual(
-    "1 an, 2 mois, 1 semaine, 3 jours, 4 heures, 5 minutes, 6 secondes, 7 millisecondes"
-  );
-});