Fix for negative values in plus/minus. Fixes moment#645

Requires the `fullConversionMatrices` and `noNormalizationInShiftTo` branches to be merged first.
GillesDebunne · Jun 7, 2020 · 4a35e73 · 4a35e73
1 parent 918cecd
commit 4a35e73
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 27 deletions.
diff --git a/src/datetime.js b/src/datetime.js
@@ -122,22 +122,23 @@ function objToTS(obj, offset, zone) {
 
 // create a new DT instance by adding a duration, adjusting for DSTs
 function adjustTime(inst, dur) {
-  const keys = Object.keys(dur.values);
-  if (keys.indexOf("milliseconds") === -1) {
-    keys.push("milliseconds");
-  }
-
-  dur = dur.shiftTo(...keys);
-
   const oPre = inst.o,
-    year = inst.c.year + dur.years,
-    month = inst.c.month + dur.months + dur.quarters * 3,
+    year = inst.c.year + Math.trunc(dur.years),
+    month = inst.c.month + Math.trunc(dur.months) + Math.trunc(dur.quarters) * 3,
     c = Object.assign({}, inst.c, {
       year,
       month,
-      day: Math.min(inst.c.day, daysInMonth(year, month)) + dur.days + dur.weeks * 7
+      day:
+        Math.min(inst.c.day, daysInMonth(year, month)) +
+        Math.trunc(dur.days) +
+        Math.trunc(dur.weeks) * 7
     }),
     millisToAdd = Duration.fromObject({
+      years: dur.years - Math.trunc(dur.years),
+      quarters: dur.quarters - Math.trunc(dur.quarters),
+      months: dur.months - Math.trunc(dur.months),
+      weeks: dur.weeks - Math.trunc(dur.weeks),
+      days: dur.days - Math.trunc(dur.days),
       hours: dur.hours,
       minutes: dur.minutes,
       seconds: dur.seconds,

diff --git a/test/datetime/math.test.js b/test/datetime/math.test.js
@@ -106,7 +106,7 @@ test("DateTime#plus renders invalid when out of max. datetime range using second
   expect(d.isValid).toBe(false);
 });
 
-test("DateTime#plus handles franctional days", () => {
+test("DateTime#plus handles fractional days", () => {
   const d = DateTime.fromISO("2016-01-31T10:00");
   expect(d.plus({ days: 0.8 })).toEqual(d.plus({ hours: (24 * 4) / 5 }));
   expect(d.plus({ days: 6.8 })).toEqual(d.plus({ days: 6, hours: (24 * 4) / 5 }));
@@ -115,14 +115,26 @@ test("DateTime#plus handles franctional days", () => {
   );
 });
 
-test("DateTime#plus handles franctional months", () => {
-  const d = DateTime.fromISO("2016-01-31T10:00");
-  expect(d.plus({ months: 8.7 })).toEqual(
-    d.plus({
-      months: 8,
-      milliseconds: Duration.fromObject({ months: 0.7 }).shiftTo("milliseconds")
-    })
-  );
+test("DateTime#plus handles fractional large units", () => {
+  const units = ["weeks", "months", "quarters", "years"];
+
+  for (const unit of units) {
+    const d = DateTime.fromISO("2016-01-31T10:00");
+    expect(d.plus({ [unit]: 8.7 })).toEqual(
+      d.plus({
+        [unit]: 8,
+        milliseconds: Duration.fromObject({ [unit]: 0.7 }).as("milliseconds")
+      })
+    );
+  }
+});
+
+// #645
+test("DateTime#plus supports positive and negative duration units", () => {
+  const d = DateTime.fromISO("2020-01-08T12:34");
+  expect(d.plus({ months: 1, days: -1 })).toEqual(d.plus({ months: 1 }).plus({ days: -1 }));
+  expect(d.plus({ years: 4, days: -1 })).toEqual(d.plus({ years: 4 }).plus({ days: -1 }));
+  expect(d.plus({ years: 0.5, days: -1.5 })).toEqual(d.plus({ years: 0.5 }).plus({ days: -1.5 }));
 });
 
 //------
@@ -183,7 +195,7 @@ test("DateTime#minus renders invalid when out of max. datetime range using secon
   expect(d.isValid).toBe(false);
 });
 
-test("DateTime#minus handles franctional days", () => {
+test("DateTime#minus handles fractional days", () => {
   const d = DateTime.fromISO("2016-01-31T10:00");
   expect(d.minus({ days: 0.8 })).toEqual(d.minus({ hours: (24 * 4) / 5 }));
   expect(d.minus({ days: 6.8 })).toEqual(d.minus({ days: 6, hours: (24 * 4) / 5 }));
@@ -192,13 +204,27 @@ test("DateTime#minus handles franctional days", () => {
   );
 });
 
-test("DateTime#minus handles franctional months", () => {
-  const d = DateTime.fromISO("2016-01-31T10:00");
-  expect(d.minus({ months: 8.7 })).toEqual(
-    d.minus({
-      months: 8,
-      milliseconds: Duration.fromObject({ months: 0.7 }).shiftTo("milliseconds")
-    })
+test("DateTime#minus handles fractional large units", () => {
+  const units = ["weeks", "months", "quarters", "years"];
+
+  for (const unit of units) {
+    const d = DateTime.fromISO("2016-01-31T10:00");
+    expect(d.minus({ [unit]: 8.7 })).toEqual(
+      d.minus({
+        [unit]: 8,
+        milliseconds: Duration.fromObject({ [unit]: 0.7 }).as("milliseconds")
+      })
+    );
+  }
+});
+
+// #645
+test("DateTime#minus supports positive and negative duration units", () => {
+  const d = DateTime.fromISO("2020-01-08T12:34");
+  expect(d.minus({ months: 1, days: -1 })).toEqual(d.minus({ months: 1 }).minus({ days: -1 }));
+  expect(d.minus({ years: 4, days: -1 })).toEqual(d.minus({ years: 4 }).minus({ days: -1 }));
+  expect(d.minus({ years: 0.5, days: -1.5 })).toEqual(
+    d.minus({ years: 0.5 }).minus({ days: -1.5 })
   );
 });