extended jsdoc about duration and units #1564
Conversation
…on#diff or Duration#shiftTo
…on#diff or Duration#shiftTo
ptandler
force-pushed
the
docs-duration-units
branch
from
c0b7eef
to
9f0220b
Compare
|
@icambron After understanding a bug I had using Luxon, I thought it might be a good idea to add a bit to the documentation so others might prevent running into the same problem. |
| * 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 |
There was a problem hiding this comment.
While I appreciate that you ran into a specific issue that was confusing, I don' t think we can really document each possible troubleshooting case in here. I think instead we should just say "Math on duration is subtle, please see https://moment.github.io/luxon/#/math".
| @@ -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, | |||
| @@ -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. | |||
There was a problem hiding this comment.
Instead, this should say something like:
Uses {@link Duration#shiftTo} to convert to the request unit. See the documentation there for details.
The reason this is better is that "unexpected" depends on your expectations. We don't want a warning message; we want a help you understand message.
| @@ -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 | |||
There was a problem hiding this comment.
Here too. Now like 95% of the doc for this is a troubleshooting guide for conversionAccuracy. I think here we just say:
Converting between units is subtle because units don't always have constant length. Please see https://moment.github.io/luxon/#/math?id=casual-vs-longterm-conversion-accuracy
especially when using Duration#diff or Duration#shiftTo