From b4c343f5a460cf466d0b64d5979d54c0992b699f Mon Sep 17 00:00:00 2001 From: Ash Date: Sat, 19 Jan 2019 02:09:44 +0000 Subject: [PATCH] [bugfix] Fix startOf/endOf DST issues while boosting performance (#4338) * Fix startOf/endOf DST issues * Performance improvements * Rewrite for legibility * Fix for years in range 0-99 * Remove placeholder comments * Revert comment * Remove useless special-case * Code review change: use break consistenly * Rename per code review * Fix typo * Inline variable to end naming debate * Address review comments --- src/lib/create/date-from-array.js | 30 +++++-- src/lib/moment/start-end-of.js | 131 +++++++++++++++++++++++------- src/test/moment/start_end_of.js | 24 ++++++ 3 files changed, 146 insertions(+), 39 deletions(-) diff --git a/src/lib/create/date-from-array.js b/src/lib/create/date-from-array.js index 59b57b0365..7fabbc05f1 100644 --- a/src/lib/create/date-from-array.js +++ b/src/lib/create/date-from-array.js @@ -1,21 +1,35 @@ export function createDate (y, m, d, h, M, s, ms) { // can't just apply() to create a date: // https://stackoverflow.com/q/181348 - var date = new Date(y, m, d, h, M, s, ms); - + var date; // the date constructor remaps years 0-99 to 1900-1999 - if (y < 100 && y >= 0 && isFinite(date.getFullYear())) { - date.setFullYear(y); + if (y < 100 && y >= 0) { + // preserve leap years using a full 400 year cycle, then reset + date = new Date(y + 400, m, d, h, M, s, ms); + if (isFinite(date.getFullYear())) { + date.setFullYear(y); + } + } else { + date = new Date(y, m, d, h, M, s, ms); } + return date; } export function createUTCDate (y) { - var date = new Date(Date.UTC.apply(null, arguments)); - + var date; // the Date.UTC function remaps years 0-99 to 1900-1999 - if (y < 100 && y >= 0 && isFinite(date.getUTCFullYear())) { - date.setUTCFullYear(y); + if (y < 100 && y >= 0) { + var args = Array.prototype.slice.call(arguments); + // preserve leap years using a full 400 year cycle, then reset + args[0] = y + 400; + date = new Date(Date.UTC.apply(null, args)); + if (isFinite(date.getUTCFullYear())) { + date.setUTCFullYear(y); + } + } else { + date = new Date(Date.UTC.apply(null, arguments)); } + return date; } diff --git a/src/lib/moment/start-end-of.js b/src/lib/moment/start-end-of.js index 02f982479a..42c19cbc3b 100644 --- a/src/lib/moment/start-end-of.js +++ b/src/lib/moment/start-end-of.js @@ -1,59 +1,128 @@ import { normalizeUnits } from '../units/aliases'; +import { hooks } from '../utils/hooks'; + +var MS_PER_SECOND = 1000; +var MS_PER_MINUTE = 60 * MS_PER_SECOND; +var MS_PER_HOUR = 60 * MS_PER_MINUTE; +var MS_PER_400_YEARS = (365 * 400 + 97) * 24 * MS_PER_HOUR; + +// actual modulo - handles negative numbers (for dates before 1970): +function mod(dividend, divisor) { + return (dividend % divisor + divisor) % divisor; +} + +function localStartOfDate(y, m, d) { + // the date constructor remaps years 0-99 to 1900-1999 + if (y < 100 && y >= 0) { + // preserve leap years using a full 400 year cycle, then reset + return new Date(y + 400, m, d) - MS_PER_400_YEARS; + } else { + return new Date(y, m, d).valueOf(); + } +} + +function utcStartOfDate(y, m, d) { + // Date.UTC remaps years 0-99 to 1900-1999 + if (y < 100 && y >= 0) { + // preserve leap years using a full 400 year cycle, then reset + return Date.UTC(y + 400, m, d) - MS_PER_400_YEARS; + } else { + return Date.UTC(y, m, d); + } +} export function startOf (units) { + var time; units = normalizeUnits(units); - // the following switch intentionally omits break keywords - // to utilize falling through the cases. + if (units === undefined || units === 'millisecond' || !this.isValid()) { + return this; + } + + var startOfDate = this._isUTC ? utcStartOfDate : localStartOfDate; + switch (units) { case 'year': - this.month(0); - /* falls through */ + time = startOfDate(this.year(), 0, 1); + break; case 'quarter': + time = startOfDate(this.year(), this.month() - this.month() % 3, 1); + break; case 'month': - this.date(1); - /* falls through */ + time = startOfDate(this.year(), this.month(), 1); + break; case 'week': + time = startOfDate(this.year(), this.month(), this.date() - this.weekday()); + break; case 'isoWeek': + time = startOfDate(this.year(), this.month(), this.date() - (this.isoWeekday() - 1)); + break; case 'day': case 'date': - this.hours(0); - /* falls through */ + time = startOfDate(this.year(), this.month(), this.date()); + break; case 'hour': - this.minutes(0); - /* falls through */ + time = this._d.valueOf(); + time -= mod(time + (this._isUTC ? 0 : this.utcOffset() * MS_PER_MINUTE), MS_PER_HOUR); + break; case 'minute': - this.seconds(0); - /* falls through */ + time = this._d.valueOf(); + time -= mod(time, MS_PER_MINUTE); + break; case 'second': - this.milliseconds(0); - } - - // weeks are a special case - if (units === 'week') { - this.weekday(0); - } - if (units === 'isoWeek') { - this.isoWeekday(1); - } - - // quarters are also special - if (units === 'quarter') { - this.month(Math.floor(this.month() / 3) * 3); + time = this._d.valueOf(); + time -= mod(time, MS_PER_SECOND); + break; } + this._d.setTime(time); + hooks.updateOffset(this, true); return this; } export function endOf (units) { + var time; units = normalizeUnits(units); - if (units === undefined || units === 'millisecond') { + if (units === undefined || units === 'millisecond' || !this.isValid()) { return this; } - // 'date' is an alias for 'day', so it should be considered as such. - if (units === 'date') { - units = 'day'; + var startOfDate = this._isUTC ? utcStartOfDate : localStartOfDate; + + switch (units) { + case 'year': + time = startOfDate(this.year() + 1, 0, 1) - 1; + break; + case 'quarter': + time = startOfDate(this.year(), this.month() - this.month() % 3 + 3, 1) - 1; + break; + case 'month': + time = startOfDate(this.year(), this.month() + 1, 1) - 1; + break; + case 'week': + time = startOfDate(this.year(), this.month(), this.date() - this.weekday() + 7) - 1; + break; + case 'isoWeek': + time = startOfDate(this.year(), this.month(), this.date() - (this.isoWeekday() - 1) + 7) - 1; + break; + case 'day': + case 'date': + time = startOfDate(this.year(), this.month(), this.date() + 1) - 1; + break; + case 'hour': + time = this._d.valueOf(); + time += MS_PER_HOUR - mod(time + (this._isUTC ? 0 : this.utcOffset() * MS_PER_MINUTE), MS_PER_HOUR) - 1; + break; + case 'minute': + time = this._d.valueOf(); + time += MS_PER_MINUTE - mod(time, MS_PER_MINUTE) - 1; + break; + case 'second': + time = this._d.valueOf(); + time += MS_PER_SECOND - mod(time, MS_PER_SECOND) - 1; + break; } - return this.startOf(units).add(1, (units === 'isoWeek' ? 'week' : units)).subtract(1, 'ms'); + this._d.setTime(time); + hooks.updateOffset(this, true); + return this; } diff --git a/src/test/moment/start_end_of.js b/src/test/moment/start_end_of.js index 4b47620d63..e3db910474 100644 --- a/src/test/moment/start_end_of.js +++ b/src/test/moment/start_end_of.js @@ -393,3 +393,27 @@ test('endOf millisecond and no-arg', function (assert) { assert.equal(+m, +m.clone().endOf('millisecond'), 'endOf with millisecond argument should change time'); assert.equal(+m, +m.clone().endOf('milliseconds'), 'endOf with milliseconds argument should change time'); }); + +test('startOf for year zero', function (assert) { + var m = moment('0000-02-29T12:34:56.789Z').parseZone(); + assert.equal(m.clone().startOf('ms').toISOString(), '0000-02-29T12:34:56.789Z', 'startOf millisecond should preserve year'); + assert.equal(m.clone().startOf('s').toISOString(), '0000-02-29T12:34:56.000Z', 'startOf second should preserve year'); + assert.equal(m.clone().startOf('m').toISOString(), '0000-02-29T12:34:00.000Z', 'startOf minute should preserve year'); + assert.equal(m.clone().startOf('h').toISOString(), '0000-02-29T12:00:00.000Z', 'startOf hour should preserve year'); + assert.equal(m.clone().startOf('d').toISOString(), '0000-02-29T00:00:00.000Z', 'startOf day should preserve year'); + assert.equal(m.clone().startOf('M').toISOString(), '0000-02-01T00:00:00.000Z', 'startOf month should preserve year'); + assert.equal(m.clone().startOf('Q').toISOString(), '0000-01-01T00:00:00.000Z', 'startOf quarter should preserve year'); + assert.equal(m.clone().startOf('y').toISOString(), '0000-01-01T00:00:00.000Z', 'startOf year should preserve year'); +}); + +test('endOf for year zero', function (assert) { + var m = moment('0000-02-29T12:34:56.789Z').parseZone(); + assert.equal(m.clone().endOf('ms').toISOString(), '0000-02-29T12:34:56.789Z', 'endOf millisecond should preserve year'); + assert.equal(m.clone().endOf('s').toISOString(), '0000-02-29T12:34:56.999Z', 'endOf second should preserve year'); + assert.equal(m.clone().endOf('m').toISOString(), '0000-02-29T12:34:59.999Z', 'endOf minute should preserve year'); + assert.equal(m.clone().endOf('h').toISOString(), '0000-02-29T12:59:59.999Z', 'endOf hour should preserve year'); + assert.equal(m.clone().endOf('d').toISOString(), '0000-02-29T23:59:59.999Z', 'endOf day should preserve year'); + assert.equal(m.clone().endOf('M').toISOString(), '0000-02-29T23:59:59.999Z', 'endOf month should preserve year'); + assert.equal(m.clone().endOf('Q').toISOString(), '0000-03-31T23:59:59.999Z', 'endOf quarter should preserve year'); + assert.equal(m.clone().endOf('y').toISOString(), '0000-12-31T23:59:59.999Z', 'endOf year should preserve year'); +});