[bugfix]: isBetween should return false for invalid dates

moment.d.ts

@@ -300,7 +300,7 @@ declare namespace moment {
 
     type DurationAs = Base;
 
-    type StartOf = Base | _quarter | _isoWeek | _date;
+    type StartOf = Base | _quarter | _isoWeek | _date | null;
 
     type Diff = Base | _quarter;
 

src/lib/moment/compare.js

@@ -1,14 +1,13 @@
 import { isMoment } from './constructor';
 import { normalizeUnits } from '../units/aliases';
 import { createLocal } from '../create/local';
-import isUndefined from '../utils/is-undefined';
 
 export function isAfter (input, units) {
     var localInput = isMoment(input) ? input : createLocal(input);
     if (!(this.isValid() && localInput.isValid())) {
         return false;
     }
-    units = normalizeUnits(!isUndefined(units) ? units : 'millisecond');
+    units = normalizeUnits(units) || 'millisecond';
     if (units === 'millisecond') {
         return this.valueOf() > localInput.valueOf();
     } else {
@@ -21,7 +20,7 @@ export function isBefore (input, units) {
     if (!(this.isValid() && localInput.isValid())) {
         return false;
     }
-    units = normalizeUnits(!isUndefined(units) ? units : 'millisecond');
+    units = normalizeUnits(units) || 'millisecond';
     if (units === 'millisecond') {
         return this.valueOf() < localInput.valueOf();
     } else {
@@ -30,9 +29,14 @@ export function isBefore (input, units) {
 }
 
 export function isBetween (from, to, units, inclusivity) {
+    var localFrom = isMoment(from) ? from : createLocal(from),
+        localTo = isMoment(to) ? to : createLocal(to);
+    if (!(this.isValid() && localFrom.isValid() && localTo.isValid())) {
+        return false;
+    }
     inclusivity = inclusivity || '()';
-    return (inclusivity[0] === '(' ? this.isAfter(from, units) : !this.isBefore(from, units)) &&
-        (inclusivity[1] === ')' ? this.isBefore(to, units) : !this.isAfter(to, units));
+    return (inclusivity[0] === '(' ? this.isAfter(localFrom, units) : !this.isBefore(localFrom, units)) &&
+        (inclusivity[1] === ')' ? this.isBefore(localTo, units) : !this.isAfter(localTo, units));
 }
 
 export function isSame (input, units) {
@@ -41,7 +45,7 @@ export function isSame (input, units) {
     if (!(this.isValid() && localInput.isValid())) {
         return false;
     }
-    units = normalizeUnits(units || 'millisecond');
+    units = normalizeUnits(units) || 'millisecond';
     if (units === 'millisecond') {
         return this.valueOf() === localInput.valueOf();
     } else {
@@ -51,9 +55,9 @@ export function isSame (input, units) {
 }
 
 export function isSameOrAfter (input, units) {
-    return this.isSame(input, units) || this.isAfter(input,units);
+    return this.isSame(input, units) || this.isAfter(input, units);
 }
 
 export function isSameOrBefore (input, units) {
-    return this.isSame(input, units) || this.isBefore(input,units);
+    return this.isSame(input, units) || this.isBefore(input, units);
 }

src/test/moment/is_between.js

@@ -226,7 +226,7 @@ test('is between year', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2010, 5, 6, 7, 8, 9, 10)),
                 moment(new Date(2011, 5, 6, 7, 8, 9, 10)), 'year'), false, 'year is later');
-    assert.equal(m.isBetween(m, 'year'), false, 'same moments are not between the same year');
+    assert.equal(m.isBetween(m, m, 'year'), false, 'same moments are not between the same year');
     assert.equal(+m, +mCopy, 'isBetween year should not change moment');
 });
 
@@ -247,7 +247,7 @@ test('is between month', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 11, 6, 7, 8, 9, 10)),
                 moment(new Date(2011, 1, 6, 7, 8, 9, 10)), 'month'), false, 'month is later');
-    assert.equal(m.isBetween(m, 'month'), false, 'same moments are not between the same month');
+    assert.equal(m.isBetween(m, m, 'month'), false, 'same moments are not between the same month');
     assert.equal(+m, +mCopy, 'isBetween month should not change moment');
 });
 
@@ -268,7 +268,7 @@ test('is between day', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 1, 7, 8, 9, 10)),
                 moment(new Date(2011, 1, 2, 7, 8, 9, 10)), 'day'), false, 'day is later');
-    assert.equal(m.isBetween(m, 'day'), false, 'same moments are not between the same day');
+    assert.equal(m.isBetween(m, m, 'day'), false, 'same moments are not between the same day');
     assert.equal(+m, +mCopy, 'isBetween day should not change moment');
 });
 
@@ -289,7 +289,7 @@ test('is between hour', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 2, 7, 8, 9, 10)),
                 moment(new Date(2011, 1, 2, 7, 8, 9, 10)), 'hour'), false, 'hour is later');
-    assert.equal(m.isBetween(m, 'hour'), false, 'same moments are not between the same hour');
+    assert.equal(m.isBetween(m, m, 'hour'), false, 'same moments are not between the same hour');
     assert.equal(+m, +mCopy, 'isBetween hour should not change moment');
 });
 
@@ -310,7 +310,7 @@ test('is between minute', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 2, 3, 2, 9, 10)),
                 moment(new Date(2011, 1, 2, 3, 3, 59, 999)), 'minute'), false, 'minute is later');
-    assert.equal(m.isBetween(m, 'minute'), false, 'same moments are not between the same minute');
+    assert.equal(m.isBetween(m, m, 'minute'), false, 'same moments are not between the same minute');
     assert.equal(+m, +mCopy, 'isBetween minute should not change moment');
 });
 
@@ -331,7 +331,7 @@ test('is between second', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 2, 3, 4, 3, 10)),
                 moment(new Date(2011, 1, 2, 3, 4, 4, 999)), 'second'), false, 'second is later');
-    assert.equal(m.isBetween(m, 'second'), false, 'same moments are not between the same second');
+    assert.equal(m.isBetween(m, m, 'second'), false, 'same moments are not between the same second');
     assert.equal(+m, +mCopy, 'isBetween second should not change moment');
 });
 
@@ -352,6 +352,16 @@ test('is between millisecond', function (assert) {
     assert.equal(m.isBetween(
                 moment(new Date(2011, 1, 2, 3, 4, 5, 4)),
                 moment(new Date(2011, 1, 2, 3, 4, 5, 6)), 'millisecond'), false, 'millisecond is later');
-    assert.equal(m.isBetween(m, 'millisecond'), false, 'same moments are not between the same millisecond');
+    assert.equal(m.isBetween(m, m, 'millisecond'), false, 'same moments are not between the same millisecond');
     assert.equal(+m, +mCopy, 'isBetween millisecond should not change moment');
 });
+
+test('is between invalid', function (assert) {
+    var invalid = moment(NaN),
+        valid = moment(new Date(2011, 1, 2, 3, 4, 5, 6)),
+        validFrom = moment(new Date(2010, 1, 2, 3, 4, 5, 6)),
+        validTo = moment(new Date(2012, 1, 2, 3, 4, 5, 6));
+    assert.equal(invalid.isBetween(validFrom, validTo), false, 'this instance invalid');
+    assert.equal(valid.isBetween(invalid, validTo), false, 'from invalid moment');
+    assert.equal(valid.isBetween(validFrom, invalid), false, 'to invalid moment');
+});

typing-tests/moment-tests.ts

@@ -222,6 +222,7 @@ moment.isDuration(moment.duration());
 moment().isBetween(moment(), moment());
 moment().isBetween(new Date(), new Date());
 moment().isBetween([1,1,2000], [1,1,2001], "year");
+moment().isBetween([1,1,2000], [1,1,2001], null, "()");
 
 moment.localeData('fr');
 moment(1316116057189).fromNow();