From c267a35fb025e84b30f51154ee37113ef9679a92 Mon Sep 17 00:00:00 2001
From: Tobias Lundin <tobias.lundin@gmail.com>
Date: Wed, 13 Apr 2022 21:30:03 +0200
Subject: [PATCH] Test code against @js-temporal/polyfill (#27)

- remove type tests
- add tests against temporal
- correct comma use
- check fractions
- throw range errors for incorrect patterns
- fix fractional hours and minutes
- refactor tests
- do not allow fractional Y/M/D
- allow mixing weeks (W) with other designators
- use correct temporal polyfill
- fix week parsing like temporal
- update readme and comments with format
- update build badge
- run tests on all push
---
 .github/workflows/pr-test.yml |  2 +-
 README.md                     | 14 +++----
 lib/index.js                  | 46 ++++++++++++---------
 package.json                  |  3 +-
 src/index.ts                  | 53 ++++++++++++++----------
 test/iso8601-tests.ts         | 76 +++++++++++++++++++++++++++++------
 test/type-tests.ts            |  9 -----
 7 files changed, 133 insertions(+), 70 deletions(-)
 delete mode 100644 test/type-tests.ts

diff --git a/.github/workflows/pr-test.yml b/.github/workflows/pr-test.yml
index d48833a..c44682d 100644
--- a/.github/workflows/pr-test.yml
+++ b/.github/workflows/pr-test.yml
@@ -1,5 +1,5 @@
 name: Run Tests
-on: [pull_request]
+on: [push]
 jobs:
   Npm-Install-And-Test:
     runs-on: ubuntu-latest
diff --git a/README.md b/README.md
index c93ba36..79f6980 100644
--- a/README.md
+++ b/README.md
@@ -2,22 +2,22 @@
 
 Node/Js-module for parsing and making sense of ISO8601-durations
 
-[![Build Status: Travis](https://img.shields.io/travis/tolu/ISO8601-duration/master.svg)][travis]
+[![Build Status](https://img.shields.io/endpoint.svg?url=https%3A%2F%2Factions-badge.atrox.dev%2Ftolu%2Fiso8601-duration%2Fbadge&style=popout)][gh-action]
 [![npm version](https://img.shields.io/npm/v/iso8601-duration.svg)][npm]
 ![npm bundle size][bundlephobia]
 
-> A new standard is on it's way, see [Temporal.Duration](https://tc39.es/proposal-temporal/docs/duration.html)
+> A new standard is on it's way, see [Temporal.Duration](https://tc39.es/proposal-temporal/docs/duration.html)  
+> Tests (most) in this module now validate against [@js-temporal/polyfill](https://www.npmjs.com/package/@js-temporal/polyfill)
 
 ## The ISO8601 duration format
 
-Durations in ISO8601 comes in two formats:
+Durations in ISO8601 comes in this string format:
 
-- **`PnYnMnDTnHnMnS`** - `P<date>T<time>`  
+- **`PnYnMnWnDTnHnMnS`** - `P<date>T<time>`  
   The `n` is replaced by the value for each of the date and time elements that follow the `n`.  
   Leading zeros are not required
-- **`PnW`** - the week format
 
-Check out the details on [Wikipedia](https://en.wikipedia.org/wiki/ISO_8601#Durations)
+Check out the details on [Wikipedia](https://en.wikipedia.org/wiki/ISO_8601#Durations) or in the coming [Temporal.Duration](https://tc39.es/proposal-temporal/docs/duration.html) spec.
 
 ## Install
 
@@ -102,6 +102,6 @@ const getWithSensibleDurations = (someApiEndpoint) => {
 
 MIT @ https://tolu.mit-license.org/
 
-[travis]: https://travis-ci.org/tolu/ISO8601-duration "travis build status"
+[gh-action]: https://actions-badge.atrox.dev/tolu/iso8601-duration/goto
 [npm]: https://www.npmjs.com/package/iso8601-duration "npm package"
 [bundlephobia]: https://img.shields.io/bundlephobia/minzip/iso8601-duration
diff --git a/lib/index.js b/lib/index.js
index 181fc7d..d9537d7 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -5,19 +5,18 @@
 Object.defineProperty(exports, "__esModule", { value: true });
 exports.toSeconds = exports.end = exports.parse = exports.pattern = void 0;
 /**
- * The pattern used for parsing ISO8601 duration (PnYnMnDTnHnMnS).
- * This does not cover the week format PnW.
+ * The pattern used for parsing ISO8601 duration (PnYnMnWnDTnHnMnS).
  */
-// PnYnMnDTnHnMnS
-var numbers = "\\d+(?:[\\.,]\\d+)?";
-var weekPattern = "(".concat(numbers, "W)");
-var datePattern = "(".concat(numbers, "Y)?(").concat(numbers, "M)?(").concat(numbers, "D)?");
-var timePattern = "T(".concat(numbers, "H)?(").concat(numbers, "M)?(").concat(numbers, "S)?");
-var iso8601 = "P(?:".concat(weekPattern, "|").concat(datePattern, "(?:").concat(timePattern, ")?)");
+// PnYnMnWnDTnHnMnS
+var numbers = "\\d+";
+var fractionalNumbers = "".concat(numbers, "(?:[\\.,]").concat(numbers, ")?");
+var datePattern = "(".concat(numbers, "Y)?(").concat(numbers, "M)?(").concat(numbers, "W)?(").concat(numbers, "D)?");
+var timePattern = "T(".concat(fractionalNumbers, "H)?(").concat(fractionalNumbers, "M)?(").concat(fractionalNumbers, "S)?");
+var iso8601 = "P(?:".concat(datePattern, "(?:").concat(timePattern, ")?)");
 var objMap = [
-    "weeks",
     "years",
     "months",
+    "weeks",
     "days",
     "hours",
     "minutes",
@@ -38,12 +37,21 @@ var defaultDuration = Object.freeze({
 exports.pattern = new RegExp(iso8601);
 /** Parse PnYnMnDTnHnMnS format to object */
 var parse = function (durationString) {
-    // Slice away first entry in match-array
-    return durationString
-        .match(exports.pattern)
-        .slice(1)
-        .reduce(function (prev, next, idx) {
-        prev[objMap[idx]] = parseFloat(next) || 0;
+    var matches = durationString.replace(/,/g, ".").match(exports.pattern);
+    if (!matches) {
+        throw new RangeError("invalid duration: ".concat(durationString));
+    }
+    // Slice away first entry in match-array (the input string)
+    var slicedMatches = matches.slice(1);
+    if (slicedMatches.filter(function (v) { return v != null; }).length === 0) {
+        throw new RangeError("invalid duration: ".concat(durationString));
+    }
+    // Check only one fraction is used
+    if (slicedMatches.filter(function (v) { return /\./.test(v || ""); }).length > 1) {
+        throw new RangeError("only the smallest unit can be fractional");
+    }
+    return slicedMatches.reduce(function (prev, next, idx) {
+        prev[objMap[idx]] = parseFloat(next || "0") || 0;
         return prev;
     }, {});
 };
@@ -58,10 +66,10 @@ var end = function (durationInput, startDate) {
     then.setFullYear(then.getFullYear() + duration.years);
     then.setMonth(then.getMonth() + duration.months);
     then.setDate(then.getDate() + duration.days);
-    then.setHours(then.getHours() + duration.hours);
-    then.setMinutes(then.getMinutes() + duration.minutes);
-    // Then.setSeconds(then.getSeconds() + duration.seconds);
-    then.setMilliseconds(then.getMilliseconds() + duration.seconds * 1000);
+    // set time as milliseconds to get fractions working for minutes/hours
+    var hoursInMs = duration.hours * 3600 * 1000;
+    var minutesInMs = duration.minutes * 60 * 1000;
+    then.setMilliseconds(then.getMilliseconds() + duration.seconds * 1000 + hoursInMs + minutesInMs);
     // Special case weeks
     then.setDate(then.getDate() + duration.weeks * 7);
     return then;
diff --git a/package.json b/package.json
index 3a6732b..f7d62a1 100644
--- a/package.json
+++ b/package.json
@@ -8,7 +8,7 @@
     "lint": "prettier --write .",
     "test": "npm run lint && npm run unittests",
     "unittests": "uvu -r ts-node/register test",
-    "watch": "onchange '**/*.js' -- npm run test",
+    "tdd": "onchange '**/*.ts' -- npm run unittests",
     "compile": "tsc",
     "prepublishOnly": "npm run compile",
     "release-patch": "npx np patch",
@@ -36,6 +36,7 @@
   "devDependencies": {
     "onchange": "^3.3.0",
     "prettier": "^2.6.2",
+    "@js-temporal/polyfill": "*",
     "ts-node": "^10.7.0",
     "typescript": "4.6.3",
     "uvu": "^0.5.3"
diff --git a/src/index.ts b/src/index.ts
index 39cdd15..bd31bda 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -3,17 +3,16 @@
  */
 
 /**
- * The pattern used for parsing ISO8601 duration (PnYnMnDTnHnMnS).
- * This does not cover the week format PnW.
+ * The pattern used for parsing ISO8601 duration (PnYnMnWnDTnHnMnS).
  */
 
-// PnYnMnDTnHnMnS
-const numbers = "\\d+(?:[\\.,]\\d+)?";
-const weekPattern = `(${numbers}W)`;
-const datePattern = `(${numbers}Y)?(${numbers}M)?(${numbers}D)?`;
-const timePattern = `T(${numbers}H)?(${numbers}M)?(${numbers}S)?`;
+// PnYnMnWnDTnHnMnS
+const numbers = "\\d+";
+const fractionalNumbers = `${numbers}(?:[\\.,]${numbers})?`;
+const datePattern = `(${numbers}Y)?(${numbers}M)?(${numbers}W)?(${numbers}D)?`;
+const timePattern = `T(${fractionalNumbers}H)?(${fractionalNumbers}M)?(${fractionalNumbers}S)?`;
 
-const iso8601 = `P(?:${weekPattern}|${datePattern}(?:${timePattern})?)`;
+const iso8601 = `P(?:${datePattern}(?:${timePattern})?)`;
 
 export interface Duration {
   years?: number;
@@ -26,9 +25,9 @@ export interface Duration {
 }
 
 const objMap: (keyof Duration)[] = [
-  "weeks",
   "years",
   "months",
+  "weeks",
   "days",
   "hours",
   "minutes",
@@ -52,14 +51,24 @@ export const pattern = new RegExp(iso8601);
 
 /** Parse PnYnMnDTnHnMnS format to object */
 export const parse = (durationString: string): Duration => {
-  // Slice away first entry in match-array
-  return durationString
-    .match(pattern)!
-    .slice(1)
-    .reduce((prev, next, idx) => {
-      prev[objMap[idx]] = parseFloat(next) || 0;
-      return prev;
-    }, {} as Duration);
+  const matches = durationString.replace(/,/g, ".").match(pattern);
+  if (!matches) {
+    throw new RangeError(`invalid duration: ${durationString}`);
+  }
+  // Slice away first entry in match-array (the input string)
+  const slicedMatches: (string | undefined)[] = matches.slice(1);
+  if (slicedMatches.filter((v) => v != null).length === 0) {
+    throw new RangeError(`invalid duration: ${durationString}`);
+  }
+  // Check only one fraction is used
+  if (slicedMatches.filter((v) => /\./.test(v || "")).length > 1) {
+    throw new RangeError(`only the smallest unit can be fractional`);
+  }
+
+  return slicedMatches.reduce((prev, next, idx) => {
+    prev[objMap[idx]] = parseFloat(next || "0") || 0;
+    return prev;
+  }, {} as Duration);
 };
 
 /** Convert ISO8601 duration object to an end Date. */
@@ -73,10 +82,12 @@ export const end = (durationInput: Duration, startDate = new Date()) => {
   then.setFullYear(then.getFullYear() + duration.years);
   then.setMonth(then.getMonth() + duration.months);
   then.setDate(then.getDate() + duration.days);
-  then.setHours(then.getHours() + duration.hours);
-  then.setMinutes(then.getMinutes() + duration.minutes);
-  // Then.setSeconds(then.getSeconds() + duration.seconds);
-  then.setMilliseconds(then.getMilliseconds() + duration.seconds * 1000);
+  // set time as milliseconds to get fractions working for minutes/hours
+  const hoursInMs = duration.hours * 3600 * 1000;
+  const minutesInMs = duration.minutes * 60 * 1000;
+  then.setMilliseconds(
+    then.getMilliseconds() + duration.seconds * 1000 + hoursInMs + minutesInMs
+  );
   // Special case weeks
   then.setDate(then.getDate() + duration.weeks * 7);
 
diff --git a/test/iso8601-tests.ts b/test/iso8601-tests.ts
index e47f7e5..4aba5ce 100644
--- a/test/iso8601-tests.ts
+++ b/test/iso8601-tests.ts
@@ -2,6 +2,70 @@ import { test } from "uvu";
 import * as assert from "uvu/assert";
 import { parse, end, toSeconds, pattern } from "../src/index";
 
+import { Temporal } from "@js-temporal/polyfill";
+
+const tryCatch = (cb) => {
+  try {
+    cb();
+  } catch (err) {
+    return err;
+  }
+  return null;
+};
+
+// needed for calendar correctness
+const relativeDate = new Date();
+// ok patterns
+[
+  "P0D",
+  "PT0S",
+  "PT0,1S", // commas as separators
+  "PT0.5M",
+  "PT0.5H",
+  "P2W2D", // Temporal allows mixing weeks with other designators
+  "PT0.001S",
+  "P1DT2H3M4S",
+  "P2Y4M6DT14H30M20.42S",
+  "P2Y4M2W6DT14H30M20.42S", // With weeks
+].forEach((value) => {
+  test(`Validate ok duration (${value}) against Temporal.Duration`, () => {
+    assert.equal(
+      toSeconds(parse(value), relativeDate),
+      Temporal.Duration.from(value).total({
+        unit: "second",
+        relativeTo: relativeDate.toISOString(),
+      }),
+      `Mismatch for pattern ${value}`
+    );
+  });
+});
+
+[
+  "abc",
+  "", // invalid duration
+  "P", // invalid duration
+  "P11", // invalid duration
+  "T", // invalid duration
+  "PT", // invalid duration
+  "P0.5Y", // invalid duration, cant have fractions in year/month/day
+  "P0.5M", // invalid duration, cant have fractions in year/month/day
+  "P0.5D", // invalid duration, cant have fractions in year/month/day
+  "PT0,2H0,1S", // only smallest number can be fractional
+].forEach((value) => {
+  test(`Validate !ok duration (${value}) against Temporal.Duration`, () => {
+    const errExpected = tryCatch(() => Temporal.Duration.from(value));
+    assert.not.equal(errExpected, null);
+    const errActual = tryCatch(() => parse(value));
+    assert.not.equal(
+      errActual,
+      null,
+      `Should have thrown: "${errExpected.toString()}"`
+    );
+    // Assert
+    assert.equal(errActual.message, errExpected.message);
+  });
+});
+
 test("Parse: correctly parses data-time format", () => {
   const time = parse("P2Y4M6DT14H30M20.42S");
   assert.is(time.years, 2);
@@ -12,18 +76,6 @@ test("Parse: correctly parses data-time format", () => {
   assert.is(time.seconds, 20.42);
 });
 
-test("Parse: correctly parses weeks format", () => {
-  const time = parse("P3W1Y4M6DT14H30M20.42S");
-
-  assert.is(time.weeks, 3);
-  assert.is(time.years, 0);
-  assert.is(time.months, 0);
-  assert.is(time.days, 0);
-  assert.is(time.hours, 0);
-  assert.is(time.minutes, 0);
-  assert.is(time.seconds, 0);
-});
-
 test("parse: allow any number of decimals", () => {
   const time = parse("PT16.239999S");
 
diff --git a/test/type-tests.ts b/test/type-tests.ts
deleted file mode 100644
index 1df518a..0000000
--- a/test/type-tests.ts
+++ /dev/null
@@ -1,9 +0,0 @@
-import iso8601, { parse, toSeconds, end, pattern } from "../src/index";
-
-// Simple typescript compile test, not actually run as a test
-
-iso8601.parse("P1Y2M4DT20H44M12.67S");
-const duration = parse("P1Y2M4DT20H44M12.67S");
-const seconds = toSeconds(duration);
-const endDate = end(duration);
-const isIso8601Date = pattern.test("P1Y2M4DT20H44M12.67S");