Repository: moment/luxon (js)

Issue:
`Duration#normalize` has trouble with negative milliseconds
**Describe the bug**
Fails to normalize a Duration with negative milliseconds.

**To Reproduce**
I was able to repro this in the playground in the docs:
```javascript
> luxon.VERSION
'2.4.0'

> let d = luxon.Duration.fromObject({seconds: -44, milliseconds: -100, minutes: 1})

> d.normalize()
Duration {values: {…}, loc: Locale, conversionAccuracy: 'casual', invalid: null, matrix: {…}, …}
...
values: {seconds: 16, milliseconds: -100, minutes: 0}
```

I would expect the above to be {seconds: 15, milliseconds: 900, minutes: 0}

Out of curiosity, I tried the initializing the "right" duration, and it has no issues normalizing:

```javascript
> let m = luxon.Duration.fromObject({seconds: 15, milliseconds: 900})

> m.normalize()
Duration {values: {…}, loc: Locale, conversionAccuracy: 'casual', invalid: null, matrix: {…}, …}
...
values: {seconds: 15, milliseconds: 900}
```


**Actual vs Expected behavior**
Since the overall duration is greater than zero, I expect all normalized fields to be greater than zero.

**Desktop (please complete the following information):**

- OS: MacOS 11.6.5
- Browser: Chrome Version 102.0.5005.115 (Official Build) (x86_64)
- Luxon version: 2.4.0 (used your playground)
- Your timezone "America/New_York"

**Additional context**
This library is excellent, by the way! 


Pull request description:
Fix normalize() to handle partially negative inputs
Continuing https://github.com/moment/luxon/pull/1296/commits/1b735939d319990e53ed440aaa3a41456488c960

Fix #1233 as well as https://github.com/moment/luxon/issues/781#issuecomment-1008051278

Values like `{ hours: 96, minutes: 0, seconds: -10 }`  will be normalized to  `{ hours: 95, minutes: 59, seconds: 50 }`

This behavior is consistent with `rescale()`

Test case provided as per https://github.com/moment/luxon/pull/1296#issuecomment-1267456483

Patch:
diff --git a/src/datetime.js b/src/datetime.js
index 90279094..40c1e6ae 100644
--- a/src/datetime.js
+++ b/src/datetime.js
@@ -894,7 +894,7 @@ export default class DateTime {
 
   /**
    * Create an invalid DateTime.
-   * @param {DateTime} reason - simple string of why this DateTime is invalid. Should not contain parameters or anything else data-dependent
+   * @param {string} reason - simple string of why this DateTime is invalid. Should not contain parameters or anything else data-dependent.
    * @param {string} [explanation=null] - longer explanation, may include parameters and other useful debugging information
    * @return {DateTime}
    */
diff --git a/src/duration.js b/src/duration.js
index 72f67fb4..49033a47 100644
--- a/src/duration.js
+++ b/src/duration.js
@@ -126,20 +126,19 @@ function clone(dur, alts, clear = false) {
   return new Duration(conf);
 }
 
-function antiTrunc(n) {
-  return n < 0 ? Math.floor(n) : Math.ceil(n);
+// this is needed since in some test cases it would return 0.9999999999999999 instead of 1
+function removePrecisionIssue(a) {
+  return Math.trunc(a * 1e3) / 1e3;
 }
 
 // NB: mutates parameters
 function convert(matrix, fromMap, fromUnit, toMap, toUnit) {
   const conv = matrix[toUnit][fromUnit],
     raw = fromMap[fromUnit] / conv,
-    sameSign = Math.sign(raw) === Math.sign(toMap[toUnit]),
-    // ok, so this is wild, but see the matrix in the tests
-    added =
-      !sameSign && toMap[toUnit] !== 0 && Math.abs(raw) <= 1 ? antiTrunc(raw) : Math.trunc(raw);
-  toMap[toUnit] += added;
-  fromMap[fromUnit] -= added * conv;
+    added = Math.floor(raw);
+
+  toMap[toUnit] = removePrecisionIssue(toMap[toUnit] + added);
+  fromMap[fromUnit] = removePrecisionIssue(fromMap[fromUnit] - added * conv);
 }
 
 // NB: mutates parameters
@@ -577,7 +576,13 @@ export default class Duration {
    * @return {number}
    */
   toMillis() {
-    return this.as("milliseconds");
+    let sum = this.values.milliseconds ?? 0;
+    for (let unit of reverseUnits.slice(1)) {
+      if (this.values?.[unit]) {
+        sum += this.values[unit] * this.matrix[unit]["milliseconds"];
+      }
+    }
+    return sum;
   }
 
   /**
@@ -694,8 +699,11 @@ export default class Duration {
   normalize() {
     if (!this.isValid) return this;
     const vals = this.toObject();
-    normalizeValues(this.matrix, vals);
-    return clone(this, { values: vals }, true);
+    if (this.valueOf() >= 0) {
+      normalizeValues(this.matrix, vals);
+      return clone(this, { values: vals }, true);
+    }
+    return this.negate().normalize().negate();
   }
 
   /**


Test patch:
diff --git a/test/duration/units.test.js b/test/duration/units.test.js
index 676f4d6f..2b37fad4 100644
--- a/test/duration/units.test.js
+++ b/test/duration/units.test.js
@@ -189,6 +189,10 @@ test("Duration#normalize handles the full grid partially negative durations", ()
       { months: 0, days: -28 },
       { months: 0, days: -28 },
     ],
+    [
+      { hours: 96, minutes: 0, seconds: -10 },
+      { hours: 95, minutes: 59, seconds: 50 },
+    ],
   ];
 
   sets.forEach(([from, to]) => {