From b736f18612bb02450fdfea8ce9b0222483be389e Mon Sep 17 00:00:00 2001 From: Ash Searle Date: Sat, 27 Jan 2018 12:29:10 +0000 Subject: [PATCH] [bugfix]: isBetween should return false for invalid dates --- moment.d.ts | 2 +- src/lib/moment/compare.js | 20 ++++++++++++-------- src/test/moment/is_between.js | 24 +++++++++++++++++------- typing-tests/moment-tests.ts | 1 + 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/moment.d.ts b/moment.d.ts index 0e74933607..49ac4c0232 100644 --- a/moment.d.ts +++ b/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; diff --git a/src/lib/moment/compare.js b/src/lib/moment/compare.js index b26bac633c..8b23dba026 100644 --- a/src/lib/moment/compare.js +++ b/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); } diff --git a/src/test/moment/is_between.js b/src/test/moment/is_between.js index b58f4ec93b..19f2b671f4 100644 --- a/src/test/moment/is_between.js +++ b/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'); +}); diff --git a/typing-tests/moment-tests.ts b/typing-tests/moment-tests.ts index bafdf11752..e21f74874f 100644 --- a/typing-tests/moment-tests.ts +++ b/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();