Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] Durations gain validity #3611

Closed
wants to merge 11 commits into from
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,5 @@ Once you become a member:
But also:
* be active in the repositories
* pick up work nobody else wants to
* attent a montly meeting
* attend a monthly meeting
* participate in the internal slack group
6 changes: 6 additions & 0 deletions src/lib/duration/as.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import { normalizeUnits } from '../units/aliases';
import toInt from '../utils/to-int';

export function as (units) {
if (!this.isValid()) {
return NaN;
}
var days;
var months;
var milliseconds = this._milliseconds;
Expand Down Expand Up @@ -31,6 +34,9 @@ export function as (units) {

// TODO: Use this.as('ms')?
export function valueOf () {
if (!this.isValid()) {
return NaN;
}
return (
this._milliseconds +
this._days * 864e5 +
Expand Down
3 changes: 3 additions & 0 deletions src/lib/duration/constructor.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { normalizeObjectUnits } from '../units/aliases';
import { getLocale } from '../locale/locales';
import isDurationValid from './valid.js';

export function Duration (duration) {
var normalizedInput = normalizeObjectUnits(duration),
Expand All @@ -13,6 +14,8 @@ export function Duration (duration) {
seconds = normalizedInput.second || 0,
milliseconds = normalizedInput.millisecond || 0;

this._isValid = isDurationValid(normalizedInput);

// representation for dateAddRemove
this._milliseconds = +milliseconds +
seconds * 1e3 + // 1000
Expand Down
2 changes: 2 additions & 0 deletions src/lib/duration/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import hasOwnProp from '../utils/has-own-prop';
import { DATE, HOUR, MINUTE, SECOND, MILLISECOND } from '../units/constants';
import { cloneWithOffset } from '../units/offset';
import { createLocal } from '../create/local';
import { createInvalid as invalid } from './valid';

// ASP.NET json date format regex
var aspNetRegex = /^(\-)?(?:(\d*)[. ])?(\d+)\:(\d+)(?:\:(\d+)(\.\d*)?)?$/;
Expand Down Expand Up @@ -77,6 +78,7 @@ export function createDuration (input, key) {
}

createDuration.fn = Duration.prototype;
createDuration.invalid = invalid;

function parseIso (inp, sign) {
// We'd normally use ~~inp for this, but unfortunately it also
Expand Down
4 changes: 2 additions & 2 deletions src/lib/duration/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import absFloor from '../utils/abs-floor';

export function get (units) {
units = normalizeUnits(units);
return this[units + 's']();
return this.isValid() ? this[units + 's']() : NaN;
}

function makeGetter(name) {
return function () {
return this._data[name];
return this.isValid() ? this._data[name] : NaN;
};
}

Expand Down
4 changes: 4 additions & 0 deletions src/lib/duration/humanize.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ export function getSetRelativeTimeThreshold (threshold, limit) {
}

export function humanize (withSuffix) {
if (!this.isValid()) {
return this.localeData().invalidDate();
}

var locale = this.localeData();
var output = relativeTime(this, !withSuffix, locale);

Expand Down
4 changes: 4 additions & 0 deletions src/lib/duration/iso-string.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ export function toISOString() {
// This is because there is no context-free conversion between hours and days
// (think of clock changes)
// and also not between days and months (28-31 days per month)
if (!this.isValid()) {
return this.localeData().invalidDate();
}

var seconds = abs(this._milliseconds) / 1000;
var days = abs(this._days);
var months = abs(this._months);
Expand Down
2 changes: 2 additions & 0 deletions src/lib/duration/prototype.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import { get, milliseconds, seconds, minutes, hours, days, months, years, weeks
import { humanize } from './humanize';
import { toISOString } from './iso-string';
import { lang, locale, localeData } from '../moment/locale';
import { isValid } from './valid';

proto.isValid = isValid;
proto.abs = abs;
proto.add = add;
proto.subtract = subtract;
Expand Down
35 changes: 35 additions & 0 deletions src/lib/duration/valid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import toInt from '../utils/to-int';
import {Duration} from './constructor';
import {createDuration} from './create';

var ordering = ['year', 'quarter', 'month', 'week', 'day', 'hour', 'minute', 'second', 'millisecond'];

export default function isDurationValid(m) {
for (var key in m) {
if (!(ordering.indexOf(key) !== -1 && (m[key] == null || !isNaN(m[key])))) {
return false;
}
}

var unitHasDecimal = false;
for (var i = 0; i < ordering.length; ++i) {
if (m[ordering[i]]) {
if (unitHasDecimal) {
return false; // only allow non-integers for smallest unit
}
if (parseFloat(m[ordering[i]]) !== toInt(m[ordering[i]])) {
unitHasDecimal = true;
}
}
}

return true;
}

export function isValid() {
return this._isValid;
}

export function createInvalid() {
return createDuration(NaN);
}
15 changes: 13 additions & 2 deletions src/test/moment/duration.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,25 @@ test('object instantiation with strings', function (assert) {

test('milliseconds instantiation', function (assert) {
assert.equal(moment.duration(72).milliseconds(), 72, 'milliseconds');
assert.equal(moment.duration(72).humanize(), 'a few seconds', 'Duration should be valid');
});

test('undefined instantiation', function (assert) {
assert.equal(moment.duration(undefined).milliseconds(), 0, 'milliseconds');
assert.equal(moment.duration(undefined).isValid(), true, '_isValid');
assert.equal(moment.duration(undefined).humanize(), 'a few seconds', 'Duration should be valid');
});

test('null instantiation', function (assert) {
assert.equal(moment.duration(null).milliseconds(), 0, 'milliseconds');
assert.equal(moment.duration(null).isValid(), true, '_isValid');
assert.equal(moment.duration(null).humanize(), 'a few seconds', 'Duration should be valid');
});

test('NaN instantiation', function (assert) {
assert.ok(isNaN(moment.duration(NaN).milliseconds()), 'milliseconds should be NaN');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is good. For tests it makes sense to add a helper method moment.duration.invalid(), the same way we have for moment, so you can properly construct an invalid duration.

assert.equal(moment.duration(NaN).isValid(), false, '_isValid');
assert.equal(moment.duration(NaN).humanize(), 'Invalid date', 'Duration should be invalid');
});

test('instantiation by type', function (assert) {
Expand Down Expand Up @@ -294,7 +305,7 @@ test('serialization to ISO 8601 duration strings', function (assert) {
assert.equal(moment.duration({M: -1}).toISOString(), '-P1M', 'one month ago');
assert.equal(moment.duration({m: -1}).toISOString(), '-PT1M', 'one minute ago');
assert.equal(moment.duration({s: -0.5}).toISOString(), '-PT0.5S', 'one half second ago');
assert.equal(moment.duration({y: -0.5, M: 1}).toISOString(), '-P5M', 'a month after half a year ago');
assert.equal(moment.duration({y: -1, M: 1}).toISOString(), '-P11M', 'a month after a year ago');
assert.equal(moment.duration({}).toISOString(), 'P0D', 'zero duration');
assert.equal(moment.duration({M: 16, d:40, s: 86465}).toISOString(), 'P1Y4M40DT24H1M5S', 'all fields');
});
Expand All @@ -304,7 +315,7 @@ test('toString acts as toISOString', function (assert) {
assert.equal(moment.duration({M: -1}).toString(), '-P1M', 'one month ago');
assert.equal(moment.duration({m: -1}).toString(), '-PT1M', 'one minute ago');
assert.equal(moment.duration({s: -0.5}).toString(), '-PT0.5S', 'one half second ago');
assert.equal(moment.duration({y: -0.5, M: 1}).toString(), '-P5M', 'a month after half a year ago');
assert.equal(moment.duration({y: -1, M: 1}).toString(), '-P11M', 'a month after a year ago');
assert.equal(moment.duration({}).toString(), 'P0D', 'zero duration');
assert.equal(moment.duration({M: 16, d:40, s: 86465}).toString(), 'P1Y4M40DT24H1M5S', 'all fields');
});
Expand Down
86 changes: 86 additions & 0 deletions src/test/moment/duration_invalid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { module, test } from '../qunit';
import moment from '../../moment';

module('invalid');

test('invalid duration', function (assert) {
var m = moment.duration.invalid(); // should be invalid
assert.equal(m.isValid(), false);
assert.ok(isNaN(m.valueOf()));
});

test('valid duration', function (assert) {
var m = moment.duration({d: null}); // should be valid, for now
assert.equal(m.isValid(), true);
assert.equal(m.valueOf(), 0);
});

test('invalid duration - only smallest unit can have decimal', function (assert) {
var m = moment.duration({'days': 3.5, 'hours': 1.1}); // should be invalid
assert.equal(m.isValid(), false);
assert.ok(isNaN(m.valueOf())); // .valueOf() returns NaN for invalid durations
});

test('valid duration - smallest unit can have decimal', function (assert) {
var m = moment.duration({'days': 3, 'hours': 1.1}); // should be valid
assert.equal(m.isValid(), true);
assert.equal(m.asHours(), 73.1);
});

test('invalid duration with two arguments', function (assert) {
var m = moment.duration(NaN, 'days');
assert.equal(m.isValid(), false);
assert.ok(isNaN(m.valueOf()));
});

test('invalid duration operations', function (assert) {
var invalids = [
moment.duration(NaN),
moment.duration(NaN, 'days'),
moment.duration.invalid()
],
i,
invalid,
valid = moment.duration();

for (i = 0; i < invalids.length; ++i) {
invalid = invalids[i];

assert.ok(!invalid.add(5, 'hours').isValid(), 'invalid.add is invalid; i=' + i);
assert.ok(!invalid.subtract(30, 'days').isValid(), 'invalid.subtract is invalid; i=' + i);
assert.ok(!invalid.abs().isValid(), 'invalid.abs is invalid; i=' + i);
assert.ok(isNaN(invalid.as('years')), 'invalid.as is NaN; i=' + i);
assert.ok(isNaN(invalid.asMilliseconds()), 'invalid.asMilliseconds is NaN; i=' + i);
assert.ok(isNaN(invalid.asSeconds()), 'invalid.asSeconds is NaN; i=' + i);
assert.ok(isNaN(invalid.asMinutes()), 'invalid.asMinutes is NaN; i=' + i);
assert.ok(isNaN(invalid.asHours()), 'invalid.asHours is NaN; i=' + i);
assert.ok(isNaN(invalid.asDays()), 'invalid.asDays is NaN; i=' + i);
assert.ok(isNaN(invalid.asWeeks()), 'invalid.asWeeks is NaN; i=' + i);
assert.ok(isNaN(invalid.asMonths()), 'invalid.asMonths is NaN; i=' + i);
assert.ok(isNaN(invalid.asYears()), 'invalid.asYears is NaN; i=' + i);
assert.ok(isNaN(invalid.valueOf()), 'invalid.valueOf is NaN; i=' + i);
assert.ok(isNaN(invalid.get('hours')), 'invalid.get is NaN; i=' + i);

assert.ok(isNaN(invalid.milliseconds()), 'invalid.milliseconds is NaN; i=' + i);
assert.ok(isNaN(invalid.seconds()), 'invalid.seconds is NaN; i=' + i);
assert.ok(isNaN(invalid.minutes()), 'invalid.minutes is NaN; i=' + i);
assert.ok(isNaN(invalid.hours()), 'invalid.hours is NaN; i=' + i);
assert.ok(isNaN(invalid.days()), 'invalid.days is NaN; i=' + i);
assert.ok(isNaN(invalid.weeks()), 'invalid.weeks is NaN; i=' + i);
assert.ok(isNaN(invalid.months()), 'invalid.months is NaN; i=' + i);
assert.ok(isNaN(invalid.years()), 'invalid.years is NaN; i=' + i);

assert.equal(invalid.humanize(),
invalid.localeData().invalidDate(),
'invalid.humanize is localized invalid duration string; i=' + i);
assert.equal(invalid.toISOString(),
invalid.localeData().invalidDate(),
'invalid.toISOString is localized invalid duration string; i=' + i);
assert.equal(invalid.toString(),
invalid.localeData().invalidDate(),
'invalid.toString is localized invalid duration string; i=' + i);
assert.equal(invalid.toJSON(), invalid.localeData().invalidDate(), 'invalid.toJSON is null; i=' + i);
assert.equal(invalid.locale(), 'en', 'invalid.locale; i=' + i);
assert.equal(invalid.localeData()._abbr, 'en', 'invalid.localeData()._abbr; i=' + i);
}
});