Repository: moment/luxon (js)

Issue:
Duration toISO produces invalid ISO string
Hey 👋 

There's some floating point madness going on that makes Duration produce invalid ISO strings. See example below.

```js
const duration = Duration.fromObject({seconds: 17, milliseconds: 548});
const isoString = duration.toISO() // -> "PT2M17.548000000000002S"
const parsedDuration = Duration.fromISO(isoString);
console.log(parsedDuration.isValid); // -> false
```

Pull request description:
Fix Duration#toISO floating point madness
This will fix #596 by handling [floating point madness](https://stackoverflow.com/questions/588004/is-floating-point-math-broken) when working with seconds and milliseconds. 

Patch:
diff --git a/benchmarks/datetime.js b/benchmarks/datetime.js
index f226857a..6b054ebd 100644
--- a/benchmarks/datetime.js
+++ b/benchmarks/datetime.js
@@ -2,6 +2,7 @@
 /* eslint no-console: off */
 import Benchmark from "benchmark";
 import DateTime from "../src/datetime";
+import Settings from "../src/settings";
 
 const suite = new Benchmark.Suite();
 
@@ -37,13 +38,20 @@ suite
   .add("DateTime#toFormat", () => {
     dt.toFormat("yyyy-MM-dd");
   })
+  .add("DateTime#toFormat with macro", () => {
+    dt.toFormat("T");
+  })
+  .add("DateTime#toFormat with macro no cache", () => {
+    dt.toFormat("T");
+    Settings.resetCaches();
+  })
   .add("DateTime#add", () => {
     dt.plus({ milliseconds: 3434 });
   })
   .add("DateTime#toISO", () => {
     dt.toISO();
   })
-  .add("DateTime#toFormatString", () => {
+  .add("DateTime#toLocaleString", () => {
     dt.toLocaleString();
   })
   .on("cycle", event => {
diff --git a/changelog.md b/changelog.md
index 7cba7a16..c57b21de 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## 1.20.0
+
+ * Added Duration#mapUnits
+ * added Interval#toISODate and Interval#toISOTime
+ * Some documentation fixes
+
 ## 1.19.3
 
  * Cache offset values
diff --git a/package.json b/package.json
index 7f84c66e..0c3209e0 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "luxon",
-  "version": "1.19.3",
+  "version": "1.20.0",
   "description": "Immutable date wrapper",
   "author": "Isaac Cambron",
   "keywords": [
diff --git a/src/duration.js b/src/duration.js
index fbef70f3..ad973ead 100644
--- a/src/duration.js
+++ b/src/duration.js
@@ -1,10 +1,17 @@
-import { isUndefined, isNumber, normalizeObject, hasOwnProperty, asNumber } from "./impl/util.js";
-import Locale from "./impl/locale.js";
+import { InvalidArgumentError, InvalidDurationError, InvalidUnitError } from "./errors.js";
 import Formatter from "./impl/formatter.js";
+import Invalid from "./impl/invalid.js";
+import Locale from "./impl/locale.js";
 import { parseISODuration } from "./impl/regexParser.js";
+import {
+  asNumber,
+  hasOwnProperty,
+  isNumber,
+  isUndefined,
+  normalizeObject,
+  roundTo
+} from "./impl/util.js";
 import Settings from "./settings.js";
-import { InvalidArgumentError, InvalidDurationError, InvalidUnitError } from "./errors.js";
-import Invalid from "./impl/invalid.js";
 
 const INVALID = "Invalid Duration";
 
@@ -416,7 +423,9 @@ export default class Duration {
     if (this.hours !== 0) s += this.hours + "H";
     if (this.minutes !== 0) s += this.minutes + "M";
     if (this.seconds !== 0 || this.milliseconds !== 0)
-      s += this.seconds + this.milliseconds / 1000 + "S";
+      // this will handle "floating point madness" by removing extra decimal places
+      // https://stackoverflow.com/questions/588004/is-floating-point-math-broken
+      s += roundTo(this.seconds + this.milliseconds / 1000, 3) + "S";
     if (s === "P") s += "T0S";
     return s;
   }
diff --git a/src/impl/locale.js b/src/impl/locale.js
index 2876f8c1..9e9381c9 100644
--- a/src/impl/locale.js
+++ b/src/impl/locale.js
@@ -16,7 +16,7 @@ function getCachedDTF(locString, opts = {}) {
 }
 
 let intlNumCache = {};
-function getCachendINF(locString, opts = {}) {
+function getCachedINF(locString, opts = {}) {
   const key = JSON.stringify([locString, opts]);
   let inf = intlNumCache[key];
   if (!inf) {
@@ -27,7 +27,7 @@ function getCachendINF(locString, opts = {}) {
 }
 
 let intlRelCache = {};
-function getCachendRTF(locString, opts = {}) {
+function getCachedRTF(locString, opts = {}) {
   const key = JSON.stringify([locString, opts]);
   let inf = intlRelCache[key];
   if (!inf) {
@@ -155,7 +155,7 @@ class PolyNumberFormatter {
     if (!forceSimple && hasIntl()) {
       const intlOpts = { useGrouping: false };
       if (opts.padTo > 0) intlOpts.minimumIntegerDigits = opts.padTo;
-      this.inf = getCachendINF(intl, intlOpts);
+      this.inf = getCachedINF(intl, intlOpts);
     }
   }
 
@@ -253,7 +253,7 @@ class PolyRelFormatter {
   constructor(intl, isEnglish, opts) {
     this.opts = Object.assign({ style: "long" }, opts);
     if (!isEnglish && hasRelative()) {
-      this.rtf = getCachendRTF(intl, opts);
+      this.rtf = getCachedRTF(intl, opts);
     }
   }
 


Test patch:
diff --git a/test/duration/format.test.js b/test/duration/format.test.js
index e0233c04..699ecdaf 100644
--- a/test/duration/format.test.js
+++ b/test/duration/format.test.js
@@ -49,6 +49,19 @@ test("Duration#toISO handles milliseconds duration", () => {
   expect(Duration.fromObject({ milliseconds: 7 }).toISO()).toBe("PT0.007S");
 });
 
+test("Duration#toISO handles seconds/milliseconds duration", () => {
+  expect(Duration.fromObject({ seconds: 17, milliseconds: 548 }).toISO()).toBe("PT17.548S");
+});
+
+test("Duration#toISO handles negative seconds/milliseconds duration", () => {
+  expect(Duration.fromObject({ seconds: -17, milliseconds: -548 }).toISO()).toBe("PT-17.548S");
+});
+
+test("Duration#toISO handles mixed negative/positive numbers in seconds/milliseconds durations", () => {
+  expect(Duration.fromObject({ seconds: 17, milliseconds: -548 }).toISO()).toBe("PT16.452S");
+  expect(Duration.fromObject({ seconds: -17, milliseconds: 548 }).toISO()).toBe("PT-16.452S");
+});
+
 //------
 // #toJSON()
 //------