From 9366c46bc926e2e746a18afbd6d5777fbe502460 Mon Sep 17 00:00:00 2001 From: Peter Tandler Date: Tue, 9 Jan 2024 15:42:11 +0100 Subject: [PATCH 1/2] extended jsdoc about duration and units, especially when using Duration#diff or Duration#shiftTo --- src/datetime.js | 23 +++++++++++++++++++---- src/duration.js | 42 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/src/datetime.js b/src/datetime.js index 9472cb531..1b17febb5 100644 --- a/src/datetime.js +++ b/src/datetime.js @@ -2031,6 +2031,13 @@ export default class DateTime { /** * Return the difference between two DateTimes as a Duration. + * + * Note that depending on the context you use the duration for, it might be necessary to pass the target units you need, + * as this defaults to milliseconds (see examples below). + * + * When you first calculate the duration as milliseconds and then in a second step shift the units, it's quite likely + * that there are cases where you get undesired results, e.g. 1 year -> 31622400000 ms -> 1 year and 6 days (see below) + * * @param {DateTime} otherDateTime - the DateTime to compare this one to * @param {string|string[]} [unit=['milliseconds']] - the unit or array of units (such as 'hours' or 'days') to include in the duration. * @param {Object} opts - options that affect the creation of the Duration @@ -2038,10 +2045,18 @@ export default class DateTime { * @example * var i1 = DateTime.fromISO('1982-05-25T09:45'), * i2 = DateTime.fromISO('1983-10-14T10:30'); - * i2.diff(i1).toObject() //=> { milliseconds: 43807500000 } - * i2.diff(i1, 'hours').toObject() //=> { hours: 12168.75 } - * i2.diff(i1, ['months', 'days']).toObject() //=> { months: 16, days: 19.03125 } - * i2.diff(i1, ['months', 'days', 'hours']).toObject() //=> { months: 16, days: 19, hours: 0.75 } + * i2.diff(i1).toObject() // => { milliseconds: 43807500000 } + * i2.diff(i1, 'hours').toObject() // => { hours: 12168.75 } + * i2.diff(i1, ['months', 'days']).toObject() // => { months: 16, days: 19.03125 } + * i2.diff(i1, ['months', 'days', 'hours']).toObject() // => { months: 16, days: 19, hours: 0.75 } + * + * var d1 = DateTime.fromISO('2001-01-01T00:00:00').diff(DateTime.fromISO('2000-01-01T00:00:00')) + * // => { milliseconds: 31622400000 } + * var d2 = d1.shiftTo('years', 'months', 'days', 'hours', 'minutes', 'seconds') + * // => { years: 1, months: 0, days: 6, hours: 0, minutes: 0, seconds: 0 } + * var d3 = DateTime.fromISO('2001-01-01T00:00:00').diff(DateTime.fromISO('2000-01-01T00:00:00'), + * ['years', 'months', 'days', 'hours', 'minutes', 'seconds']) + * // => { years: 1, months: 0, days: 0, hours: 0, minutes: 0, seconds: 0 } * @return {Duration} */ diff(otherDateTime, unit = "milliseconds", opts = {}) { diff --git a/src/duration.js b/src/duration.js index c1135defe..78b307f98 100644 --- a/src/duration.js +++ b/src/duration.js @@ -257,7 +257,8 @@ export default class Duration { * @param {Object} opts - options for parsing * @param {string} [opts.locale='en-US'] - the locale to use * @param {string} opts.numberingSystem - the numbering system to use - * @param {string} [opts.conversionAccuracy='casual'] - the conversion system to use + * @param {string} [opts.conversionAccuracy='casual'] - the conversion system to use, + * see https://moment.github.io/luxon/#/math * @return {Duration} */ static fromMillis(count, opts) { @@ -280,7 +281,8 @@ export default class Duration { * @param {Object} [opts=[]] - options for creating this Duration * @param {string} [opts.locale='en-US'] - the locale to use * @param {string} opts.numberingSystem - the numbering system to use - * @param {string} [opts.conversionAccuracy='casual'] - the preset conversion system to use + * @param {string} [opts.conversionAccuracy='casual'] - the preset conversion system to use, + * see https://moment.github.io/luxon/#/math * @param {string} [opts.matrix=Object] - the custom conversion system to use * @return {Duration} */ @@ -331,7 +333,8 @@ export default class Duration { * @param {Object} opts - options for parsing * @param {string} [opts.locale='en-US'] - the locale to use * @param {string} opts.numberingSystem - the numbering system to use - * @param {string} [opts.conversionAccuracy='casual'] - the preset conversion system to use + * @param {string} [opts.conversionAccuracy='casual'] - the preset conversion system to use, + * see https://moment.github.io/luxon/#/math * @param {string} [opts.matrix=Object] - the preset conversion system to use * @see https://en.wikipedia.org/wiki/ISO_8601#Durations * @example Duration.fromISO('P3Y6M1W4DT12H30M5S').toObject() //=> { years: 3, months: 6, weeks: 1, days: 4, hours: 12, minutes: 30, seconds: 5 } @@ -354,7 +357,8 @@ export default class Duration { * @param {Object} opts - options for parsing * @param {string} [opts.locale='en-US'] - the locale to use * @param {string} opts.numberingSystem - the numbering system to use - * @param {string} [opts.conversionAccuracy='casual'] - the preset conversion system to use + * @param {string} [opts.conversionAccuracy='casual'] - the preset conversion system to use, + * see https://moment.github.io/luxon/#/math * @param {string} [opts.matrix=Object] - the conversion system to use * @see https://en.wikipedia.org/wiki/ISO_8601#Times * @example Duration.fromISOTime('11:22:33.444').toObject() //=> { hours: 11, minutes: 22, seconds: 33, milliseconds: 444 } @@ -725,6 +729,9 @@ export default class Duration { /** * Return the length of the duration in the specified unit. + * + * Note as this uses {@link Duration#shiftTo} internally, it can cause the unexpected effects as described there. + * * @param {string} unit - a unit such as 'minutes' or 'days' * @example Duration.fromObject({years: 1}).as('days') //=> 365 * @example Duration.fromObject({years: 1}).as('months') //=> 12 @@ -759,6 +766,9 @@ export default class Duration { /** * Rescale units to its largest representation + * + * Note as this uses {@link Duration#shiftTo} internally, it can cause the unexpected effects as described there. + * * @example Duration.fromObject({ milliseconds: 90000 }).rescale().toObject() //=> { minutes: 1, seconds: 30 } * @return {Duration} */ @@ -770,7 +780,25 @@ export default class Duration { /** * Convert this Duration into its representation in a different set of units. - * @example Duration.fromObject({ hours: 1, seconds: 30 }).shiftTo('minutes', 'milliseconds').toObject() //=> { minutes: 60, milliseconds: 30000 } + * + * Note that using shiftTo() can sometimes have unexpected results when using units that don't refer to a + * constant duration, such as months or years (see example below). In these cases shiftTo() uses a constant factor + * to calculate with these units as described at https://moment.github.io/luxon/#/math?id=casual-vs-longterm-conversion-accuracy + * + * If you can't avoid using shiftTo() in these cases, you can choose which conversion scheme is appropriate in + * your case which you can pass when constructing a Duration with the `conversionAccuracy` option, + * see e.g. {@link Duration.fromObject}. + * If you construct a Duration using {@link Duration#diff}, you can pass the desired target units directly. + * + * @example Duration.fromObject({ hours: 1, seconds: 30 }).shiftTo('minutes', 'milliseconds').toObject() // => { minutes: 60, milliseconds: 30000 } + * + * @example + * var d1 = DateTime.fromISO('2001-01-01T00:00:00').diff(DateTime.fromISO('2000-01-01T00:00:00')) + * // => { milliseconds: 31622400000 } + * var d2 = d1.shiftTo('years', 'months', 'days', 'hours', 'minutes', 'seconds') + * // => { years: 1, months: 0, days: 6, hours: 0, minutes: 0, seconds: 0 } + * // Oops, we got 6 days more than we had before! + * * @return {Duration} */ shiftTo(...units) { @@ -832,6 +860,10 @@ export default class Duration { /** * Shift this Duration to all available units. * Same as shiftTo("years", "months", "weeks", "days", "hours", "minutes", "seconds", "milliseconds") + * + * Note as this uses {@link Duration#shiftTo} internally, it can cause the unexpected effects as described there. + * + * @see Duration#shiftTo * @return {Duration} */ shiftToAll() { From 9f0220b22d4e19c4058d69449679db55d1a7a650 Mon Sep 17 00:00:00 2001 From: Peter Tandler Date: Tue, 9 Jan 2024 15:43:04 +0100 Subject: [PATCH 2/2] extended jsdoc about duration and units, especially when using Duration#diff or Duration#shiftTo --- src/datetime.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/datetime.js b/src/datetime.js index 1b17febb5..8dc190d5e 100644 --- a/src/datetime.js +++ b/src/datetime.js @@ -2038,6 +2038,8 @@ export default class DateTime { * When you first calculate the duration as milliseconds and then in a second step shift the units, it's quite likely * that there are cases where you get undesired results, e.g. 1 year -> 31622400000 ms -> 1 year and 6 days (see below) * + * For details about calculation with duration units, see https://moment.github.io/luxon/#/math + * * @param {DateTime} otherDateTime - the DateTime to compare this one to * @param {string|string[]} [unit=['milliseconds']] - the unit or array of units (such as 'hours' or 'days') to include in the duration. * @param {Object} opts - options that affect the creation of the Duration @@ -2057,6 +2059,7 @@ export default class DateTime { * var d3 = DateTime.fromISO('2001-01-01T00:00:00').diff(DateTime.fromISO('2000-01-01T00:00:00'), * ['years', 'months', 'days', 'hours', 'minutes', 'seconds']) * // => { years: 1, months: 0, days: 0, hours: 0, minutes: 0, seconds: 0 } + * @see Duration#shiftTo * @return {Duration} */ diff(otherDateTime, unit = "milliseconds", opts = {}) {